feature: use hypothetical transcript snippet for the query#15
Conversation
📝 WalkthroughWalkthroughAdds HyDE (Hypothetical Document Embeddings) query transformation to the retrieval pipeline via a new ChangesHyDE Retrieval Integration
LLM Provider Hardening and Dependency
Example Script and Misc Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Retriever
participant LLM
participant VectorStore
Caller->>Retriever: retrieve(query)
alt use_hyde=True
Retriever->>LLM: _generate_hypothetical_document(query)
LLM-->>Retriever: hypothetical_text
else use_hyde=False or error
Retriever->>Retriever: use original query
end
Retriever->>VectorStore: embed + search(search_query)
VectorStore-->>Retriever: results
Retriever-->>Caller: retrieved docs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
example/main.py (2)
17-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant explicit default kwargs in
finance_agent.
marketing_agentrelies on defaults for guardrails and handoffs. Match that style infinance_agentfor consistency:finance_agent = Agent( name="Finance Specialist", instructions="You answer finance and tax questions clearly.", model="gemma4", - input_guardrails=[], - output_guardrails=[], - handoffs=[], )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/main.py` around lines 17 - 24, The finance_agent initialization includes redundant explicit default keyword arguments for input_guardrails, output_guardrails, and handoffs. Update the Agent(...) call for finance_agent to match marketing_agent’s style by omitting those default kwargs and relying on the Agent defaults, keeping only the non-default configuration like name, instructions, and model.
8-13: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAdd input validation to
calculate_gst.Negative amounts would produce nonsensical negative GST. Consider guarding the input:
if amount < 0: raise ValueError("Amount must be non-negative")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/main.py` around lines 8 - 13, The calculate_gst function currently accepts negative amounts and returns nonsensical tax values. Update calculate_gst to validate its amount input at the start and raise a ValueError when the amount is negative, while keeping the existing GST and total calculation logic unchanged for valid inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 90-91: The sample environment currently enables HyDE by default,
which overrides the disabled default in Settings and adds an extra LLM call for
anyone copying the example. Update the USE_HYDE entry in the example env so it
remains opt-in, and make sure the file still ends with a final newline to
satisfy dotenv-linter. Use the USE_HYDE setting in the sample env as the target
for this change.
In `@example/main.py`:
- Line 59: The main() function is missing a return type annotation, so update
async def main() to include an explicit typed return value consistent with the
function’s behavior. Use the main function symbol itself to locate the change
and ensure it is fully typed per the project’s coding guidelines.
- Around line 71-72: Replace the direct print in the main execution flow with a
logging call and add explicit error handling around the Runner.run invocation.
Update the logic near the main_agent/Runner.run path to catch failures from the
external call, log the error through the module’s logger, and avoid printing
final_output directly. Keep the success path using logging as well so all output
and failures are handled consistently.
In `@llm/factory.py`:
- Around line 95-100: The Ollama provider setup in ChatOllama is missing the
streaming flag, which breaks parity with the OpenAI and Anthropic factory paths.
Update the create/return path in factory.py to pass streaming=True when
constructing ChatOllama, keeping the provider contract consistent for any
callback or UI flow that relies on streamed tokens.
- Around line 70-71: The ChatOllama initialization in the factory currently
passes a top-level timeout kwarg that this client does not accept, so update the
ChatOllama setup to remove that argument and place the timeout inside the
underlying client configuration instead. Keep the existing retry behavior
intact, and use the factory path that constructs ChatOllama as the place to
adjust the kwargs so it remains compatible while ChatOpenAI and ChatAnthropic
can still use their own accepted parameters.
In `@rag/retriever.py`:
- Around line 134-136: The HyDE query generation in retriever.py currently
returns whitespace-only LLM output as a valid search query, which prevents the
existing fallback from using the original query. Update the logic around the
response.content handling in the retriever’s LLM invocation path to detect empty
or whitespace-only content and raise an error instead of returning it, so the
existing HyDE fallback path can continue with the original query.
- Around line 67-83: The Retriever.retrieve flow is logging sensitive query and
HyDE content and has grown too large with nested HyDE logic. Move the HyDE
query-building branch into a dedicated helper (for example, a private method
near _generate_hypothetical_document) and have retrieve() call it before
embedding. Remove the debug/warning messages that print query[:60] and
search_query[:100], and keep logs limited to non-sensitive status information
while preserving the fallback behavior when HyDE generation fails.
In `@ui/utils.py`:
- Line 16: The api_get() helper is annotated with a false None return contract
even though it returns parsed JSON like api_post(), so update both helpers’
return annotations to match the actual r.json() payload and keep the signatures
aligned. While fixing api_get(), also tighten the **kwargs: Any usage by
replacing it with the specific forwarded HTTPX arguments accepted by the helper
or documenting why the open-ended escape hatch is required. Use the api_get and
api_post function definitions in ui/utils.py to locate the change.
---
Nitpick comments:
In `@example/main.py`:
- Around line 17-24: The finance_agent initialization includes redundant
explicit default keyword arguments for input_guardrails, output_guardrails, and
handoffs. Update the Agent(...) call for finance_agent to match
marketing_agent’s style by omitting those default kwargs and relying on the
Agent defaults, keeping only the non-default configuration like name,
instructions, and model.
- Around line 8-13: The calculate_gst function currently accepts negative
amounts and returns nonsensical tax values. Update calculate_gst to validate its
amount input at the start and raise a ValueError when the amount is negative,
while keeping the existing GST and total calculation logic unchanged for valid
inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be005730-ff76-4f73-bed4-1896226b9ad7
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.env.examplearchitecture_plan.mdconfig.pyexample/main.pyllm/factory.pypyproject.tomlrag/retriever.pyui/utils.py
| # Query transformation technique | ||
| USE_HYDE=True No newline at end of file |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Keep HyDE opt-in in the sample env.
USE_HYDE=True means anyone copying .env.example will enable an extra LLM call on every retrieval, bypassing the disabled default in Settings. If HyDE is meant to be opt-in, set this to False; also keep the final newline flagged by dotenv-linter.
Proposed fix
# Query transformation technique
-USE_HYDE=True
+USE_HYDE=False
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Query transformation technique | |
| USE_HYDE=True | |
| # Query transformation technique | |
| USE_HYDE=False | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 91-91: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 90 - 91, The sample environment currently enables
HyDE by default, which overrides the disabled default in Settings and adds an
extra LLM call for anyone copying the example. Update the USE_HYDE entry in the
example env so it remains opt-in, and make sure the file still ends with a final
newline to satisfy dotenv-linter. Use the USE_HYDE setting in the sample env as
the target for this change.
Source: Linters/SAST tools
| ) | ||
|
|
||
|
|
||
| async def main(): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add return type annotation to main().
As per coding guidelines, all functions must be fully typed.
-async def main():
+async def main() -> None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def main(): | |
| async def main() -> None: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/main.py` at line 59, The main() function is missing a return type
annotation, so update async def main() to include an explicit typed return value
consistent with the function’s behavior. Use the main function symbol itself to
locate the change and ensure it is fully typed per the project’s coding
guidelines.
Source: Coding guidelines
| result = await Runner.run(main_agent, enriched_query, run_config=run_config) | ||
| print(result.final_output) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace print with logging and add error handling.
As per coding guidelines, use logging, not print. Also wrap the external call in explicit error handling:
+import logging
+
+logger = logging.getLogger(__name__)
+
async def main() -> None:
...
- result = await Runner.run(main_agent, enriched_query, run_config=run_config)
- print(result.final_output)
+ try:
+ result = await Runner.run(main_agent, enriched_query, run_config=run_config)
+ logger.info(result.final_output)
+ except Exception:
+ logger.exception("Agent execution failed")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| result = await Runner.run(main_agent, enriched_query, run_config=run_config) | |
| print(result.final_output) | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| async def main() -> None: | |
| ... | |
| try: | |
| result = await Runner.run(main_agent, enriched_query, run_config=run_config) | |
| logger.info(result.final_output) | |
| except Exception: | |
| logger.exception("Agent execution failed") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/main.py` around lines 71 - 72, Replace the direct print in the main
execution flow with a logging call and add explicit error handling around the
Runner.run invocation. Update the logic near the main_agent/Runner.run path to
catch failures from the external call, log the error through the module’s
logger, and avoid printing final_output directly. Keep the success path using
logging as well so all output and failures are handled consistently.
Source: Coding guidelines
| timeout=120, | ||
| max_retries=3, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For langchain-openai 0.1.0, langchain-anthropic 0.1.0, and langchain-ollama 0.1.0, which constructor keyword arguments are supported for ChatOpenAI, ChatAnthropic, and ChatOllama? Specifically verify timeout, max_retries, and num_predict.
💡 Result:
For the specified versions of the LangChain partner packages, the support for timeout, max_retries, and num_predict varies by model class as follows: ChatOpenAI (langchain-openai 0.1.0) - timeout: Supported via the timeout constructor argument (alias for request_timeout), which accepts float, tuple[float, float], or None [1][2]. - max_retries: Supported via the max_retries constructor argument [1][2]. - num_predict: Not a standard constructor argument for ChatOpenAI; control max tokens via max_tokens [1]. ChatAnthropic (langchain-anthropic 0.1.0) - timeout: Supported via the timeout constructor argument (alias for default_request_timeout), which accepts a float [3][4]. - max_retries: Supported via the max_retries constructor argument [3][4]. - num_predict: Not a standard constructor argument; control output length via max_tokens [3]. ChatOllama (langchain-ollama 0.1.0) - timeout: Not a top-level constructor argument (use client_kwargs to configure underlying client timeouts if necessary) [5][6]. - max_retries: Not a standard top-level constructor argument for ChatOllama [5][6]. - num_predict: Supported via the num_predict constructor argument [5][7]. In version 0.1.0 of these packages, the initialization signatures align with their respective API client requirements, meaning Ollama-specific parameters (like num_predict) are exposed in ChatOllama, while retry and request timeout logic is explicitly managed in the OpenAI and Anthropic integrations [2][3][7].
Citations:
- 1: https://reference.langchain.com/python/langchain-openai/chat_models/base/ChatOpenAI
- 2: https://github.com/langchain-ai/langchain/blob/fd685fb779555756e1ca9fb9b0d95bee97b0602c/libs/partners/openai/langchain_openai/chat_models/base.py
- 3: https://raw.githubusercontent.com/langchain-ai/langchain/273d282a/libs/partners/anthropic/langchain_anthropic/chat_models.py
- 4: langchain-ai/langchain@3d7ae8b
- 5: https://reference.langchain.com/python/langchain-ollama/langchain_ollama/chat_models/ChatOllama
- 6: https://reference.langchain.com/python/langchain-ollama/chat_models/ChatOllama
- 7: https://github.com/langchain-ai/langchain/blob/master/libs/partners/ollama/langchain_ollama/chat_models.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant range with line numbers.
git ls-files llm/factory.py
wc -l llm/factory.py
sed -n '55,110p' llm/factory.py | cat -nRepository: varit05/Streaming-Video-RAG
Length of output: 1962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the installed/declared partner package pins and where the factory imports the adapter classes.
rg -n "langchain-(openai|anthropic|ollama)|Chat(OpenAI|Anthropic|Ollama)" -S pyproject.toml poetry.lock requirements*.txt uv.lock llm/factory.pyRepository: varit05/Streaming-Video-RAG
Length of output: 1439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the locked versions for the partner packages in uv.lock.
python3 - <<'PY'
from pathlib import Path
import re
text = Path('uv.lock').read_text()
for name in ['langchain-openai', 'langchain-anthropic', 'langchain-ollama']:
m = re.search(rf'name = "{re.escape(name)}"\nversion = "([^"]+)"', text)
print(name, m.group(1) if m else 'NOT FOUND')
PYRepository: varit05/Streaming-Video-RAG
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact factory code around the Ollama constructor for the problematic kwargs.
sed -n '37,48p' llm/factory.py | cat -nRepository: varit05/Streaming-Video-RAG
Length of output: 675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Pull the exact locked versions for the partner packages from uv.lock.
python3 - <<'PY'
from pathlib import Path
import re
text = Path('uv.lock').read_text()
for name in ['langchain-openai', 'langchain-anthropic', 'langchain-ollama']:
m = re.search(rf'name = "{re.escape(name)}"\nversion = "([^"]+)"', text)
print(f"{name}: {m.group(1) if m else 'NOT FOUND'}")
PYRepository: varit05/Streaming-Video-RAG
Length of output: 240
🌐 Web query:
For langchain-openai 1.2.2, langchain-anthropic 1.4.4, and langchain-ollama 1.1.0, which constructor keyword arguments are supported for ChatOpenAI, ChatAnthropic, and ChatOllama? Specifically verify timeout, max_retries, and num_predict, and whether ChatOllama accepts a top-level timeout.
💡 Result:
The constructor keyword arguments for the specified versions of langchain-openai, langchain-anthropic, and langchain-ollama are summarized below. ChatOpenAI (langchain-openai 1.2.2) The ChatOpenAI constructor supports both timeout and max_retries as top-level arguments [1][2]. - timeout: Accepts a float, tuple of two floats, or Any, which defines the timeout for API requests [1][2]. - max_retries: Accepts an integer or None to specify the maximum number of retries for request failures [1][2]. ChatAnthropic (langchain-anthropic 1.4.4) The ChatAnthropic constructor supports both timeout and max_retries as top-level arguments [3][4][5]. - timeout: Defined via the default_request_timeout attribute (aliased to timeout) [5]. It accepts a float or None [3]. - max_retries: Accepts an integer (defaulting to 2) to specify the number of retries allowed for requests to the Claude API [5][6]. ChatOllama (langchain-ollama 1.1.0) The ChatOllama constructor supports num_predict, but does not natively expose a top-level timeout argument in the same manner as the OpenAI or Anthropic integrations [7][8]. - num_predict: Supported as a top-level constructor argument (an integer or None), representing the maximum number of tokens to generate [7][8]. - timeout: ChatOllama does not accept a top-level timeout argument [7][8]. Timeouts for Ollama are typically managed through client_kwargs, which are passed to the underlying HTTP client (e.g., via httpx configuration) [7]. - max_retries: Not explicitly listed as a primary top-level constructor argument for ChatOllama in the Python reference documentation [7].
Citations:
- 1: https://reference.langchain.com/python/langchain-openai/chat_models/base/ChatOpenAI
- 2: https://reference.langchain.com/python/langchain-openai/langchain_openai/chat_models/base/ChatOpenAI
- 3: https://reference.langchain.com/python/langchain-anthropic/langchain_anthropic/chat_models/ChatAnthropic
- 4: https://reference.langchain.com/python/langchain-anthropic/chat_models/ChatAnthropic
- 5: https://github.com/langchain-ai/langchain/blob/273d282a/libs/partners/anthropic/langchain_anthropic/chat_models.py
- 6: https://reference.langchain.com/python/langchain-anthropic/chat_models/ChatAnthropic/max_retries
- 7: https://reference.langchain.com/python/langchain-ollama/langchain_ollama/chat_models/ChatOllama
- 8: https://github.com/langchain-ai/langchain/blob/master/libs/partners/ollama/langchain_ollama/chat_models.py
Remove the top-level timeout from ChatOllama
ChatOpenAI and ChatAnthropic accept these kwargs, but ChatOllama does not take a top-level timeout; this will fail at initialization. Move the timeout into the underlying client config instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@llm/factory.py` around lines 70 - 71, The ChatOllama initialization in the
factory currently passes a top-level timeout kwarg that this client does not
accept, so update the ChatOllama setup to remove that argument and place the
timeout inside the underlying client configuration instead. Keep the existing
retry behavior intact, and use the factory path that constructs ChatOllama as
the place to adjust the kwargs so it remains compatible while ChatOpenAI and
ChatAnthropic can still use their own accepted parameters.
| return ChatOllama( | ||
| model=settings.ollama_model, | ||
| temperature=temperature, | ||
| base_url=settings.ollama_base_url, | ||
| num_predict=4096, | ||
| timeout=300, # 5 minute timeout for Ollama |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore streaming=True on the Ollama provider.
Line 95 now builds ChatOllama(...) without the streaming flag used on the OpenAI and Anthropic paths. That changes the provider contract and can break any callback/UI flow that expects streamed tokens when switching providers.
Suggested fix
return ChatOllama(
model=settings.ollama_model,
temperature=temperature,
base_url=settings.ollama_base_url,
+ streaming=True,
num_predict=4096,
timeout=300, # 5 minute timeout for Ollama
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ChatOllama( | |
| model=settings.ollama_model, | |
| temperature=temperature, | |
| base_url=settings.ollama_base_url, | |
| num_predict=4096, | |
| timeout=300, # 5 minute timeout for Ollama | |
| return ChatOllama( | |
| model=settings.ollama_model, | |
| temperature=temperature, | |
| base_url=settings.ollama_base_url, | |
| streaming=True, | |
| num_predict=4096, | |
| timeout=300, # 5 minute timeout for Ollama |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@llm/factory.py` around lines 95 - 100, The Ollama provider setup in
ChatOllama is missing the streaming flag, which breaks parity with the OpenAI
and Anthropic factory paths. Update the create/return path in factory.py to pass
streaming=True when constructing ChatOllama, keeping the provider contract
consistent for any callback or UI flow that relies on streamed tokens.
| f"[Retriever] Query='{query[:60]}', top_k={top_k}, video_id={video_id}, use_hyde={settings.use_hyde}" | ||
| ) | ||
|
|
||
| try: | ||
| query_vector = self.embedder.embed_query(query) | ||
| # ── HyDE Transformation ─────────────────────────────────────────── | ||
| search_query = query | ||
| if settings.use_hyde: | ||
| try: | ||
| search_query = self._generate_hypothetical_document(query) | ||
| logger.debug(f"[Retriever] HyDE generated: {search_query[:100]}...") | ||
| except Exception as e: | ||
| logger.warning( | ||
| f"[Retriever] HyDE failed, falling back to original query: {e}" | ||
| ) | ||
|
|
||
| # ── Embedding & Search ─────────────────────────────────────────── | ||
| query_vector = self.embedder.embed_query(search_query) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don’t log query/HyDE content; extract HyDE query building.
query[:60] and search_query[:100] can expose user questions or transcript content in logs. Moving this branch into a helper also keeps retrieve() from growing past the project’s function-size limit. As per coding guidelines, Functions should be ≤ 50 lines, max 3 levels of nesting.
Proposed fix
logger.debug(
- f"[Retriever] Query='{query[:60]}', top_k={top_k}, video_id={video_id}, use_hyde={settings.use_hyde}"
+ f"[Retriever] query_length={len(query)}, top_k={top_k}, "
+ f"video_id={video_id}, use_hyde={settings.use_hyde}"
)
try:
- # ── HyDE Transformation ───────────────────────────────────────────
- search_query = query
- if settings.use_hyde:
- try:
- search_query = self._generate_hypothetical_document(query)
- logger.debug(f"[Retriever] HyDE generated: {search_query[:100]}...")
- except Exception as e:
- logger.warning(
- f"[Retriever] HyDE failed, falling back to original query: {e}"
- )
+ search_query = self._get_search_query(query)
# ── Embedding & Search ───────────────────────────────────────────
query_vector = self.embedder.embed_query(search_query)+ def _get_search_query(self, query: str) -> str:
+ if not settings.use_hyde:
+ return query
+ try:
+ search_query = self._generate_hypothetical_document(query)
+ logger.debug(f"[Retriever] HyDE generated {len(search_query)} characters")
+ return search_query
+ except Exception as e:
+ logger.warning(f"[Retriever] HyDE failed, falling back to original query: {e}")
+ return query
+
def format_context(self, results: list[SearchResult]) -> str:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| f"[Retriever] Query='{query[:60]}', top_k={top_k}, video_id={video_id}, use_hyde={settings.use_hyde}" | |
| ) | |
| try: | |
| query_vector = self.embedder.embed_query(query) | |
| # ── HyDE Transformation ─────────────────────────────────────────── | |
| search_query = query | |
| if settings.use_hyde: | |
| try: | |
| search_query = self._generate_hypothetical_document(query) | |
| logger.debug(f"[Retriever] HyDE generated: {search_query[:100]}...") | |
| except Exception as e: | |
| logger.warning( | |
| f"[Retriever] HyDE failed, falling back to original query: {e}" | |
| ) | |
| # ── Embedding & Search ─────────────────────────────────────────── | |
| query_vector = self.embedder.embed_query(search_query) | |
| f"[Retriever] query_length={len(query)}, top_k={top_k}, " | |
| f"video_id={video_id}, use_hyde={settings.use_hyde}" | |
| ) | |
| try: | |
| search_query = self._get_search_query(query) | |
| # ── Embedding & Search ─────────────────────────────────────────── | |
| query_vector = self.embedder.embed_query(search_query) | |
| def _get_search_query(self, query: str) -> str: | |
| if not settings.use_hyde: | |
| return query | |
| try: | |
| search_query = self._generate_hypothetical_document(query) | |
| logger.debug(f"[Retriever] HyDE generated {len(search_query)} characters") | |
| return search_query | |
| except Exception as e: | |
| logger.warning(f"[Retriever] HyDE failed, falling back to original query: {e}") | |
| return query | |
| def format_context(self, results: list[SearchResult]) -> str: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rag/retriever.py` around lines 67 - 83, The Retriever.retrieve flow is
logging sensitive query and HyDE content and has grown too large with nested
HyDE logic. Move the HyDE query-building branch into a dedicated helper (for
example, a private method near _generate_hypothetical_document) and have
retrieve() call it before embedding. Remove the debug/warning messages that
print query[:60] and search_query[:100], and keep logs limited to non-sensitive
status information while preserving the fallback behavior when HyDE generation
fails.
Source: Coding guidelines
| response = llm.invoke(messages) | ||
| content = response.content if hasattr(response, "content") else str(response) | ||
| return str(content).strip() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback when HyDE returns empty content.
A whitespace-only LLM response currently becomes the embedded search query. Raise on empty output so the existing HyDE fallback path uses the original query.
Proposed fix
response = llm.invoke(messages)
content = response.content if hasattr(response, "content") else str(response)
- return str(content).strip()
+ document = str(content).strip()
+ if not document:
+ raise ValueError("HyDE returned empty content")
+ return document📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| response = llm.invoke(messages) | |
| content = response.content if hasattr(response, "content") else str(response) | |
| return str(content).strip() | |
| response = llm.invoke(messages) | |
| content = response.content if hasattr(response, "content") else str(response) | |
| document = str(content).strip() | |
| if not document: | |
| raise ValueError("HyDE returned empty content") | |
| return document |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rag/retriever.py` around lines 134 - 136, The HyDE query generation in
retriever.py currently returns whitespace-only LLM output as a valid search
query, which prevents the existing fallback from using the original query.
Update the logic around the response.content handling in the retriever’s LLM
invocation path to detect empty or whitespace-only content and raise an error
instead of returning it, so the existing HyDE fallback path can continue with
the original query.
|
|
||
|
|
||
| def api_get(path: str, **kwargs: Any) -> Any: | ||
| def api_get(path: str, **kwargs: Any) -> None: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites that depend on api_get/api_post return values:"
rg -n -C2 '\bapi_get\s*\(' ui
rg -n -C2 '\bapi_post\s*\(' ui
echo
echo "Type-checking rules mentioned in the repo guidelines should catch this once the signature is fixed:"
echo " uvx ruff check ."
echo " uvx mypy ."Repository: varit05/Streaming-Video-RAG
Length of output: 3865
Restore the JSON return type on these helpers.
api_get() and api_post() still return r.json(), so -> None is a false contract and will trigger strict type errors wherever callers read the response. api_get() also keeps **kwargs: Any without justification; narrow it to the forwarded HTTPX args or document the escape hatch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/utils.py` at line 16, The api_get() helper is annotated with a false None
return contract even though it returns parsed JSON like api_post(), so update
both helpers’ return annotations to match the actual r.json() payload and keep
the signatures aligned. While fixing api_get(), also tighten the **kwargs: Any
usage by replacing it with the specific forwarded HTTPX arguments accepted by
the helper or documenting why the open-ended escape hatch is required. Use the
api_get and api_post function definitions in ui/utils.py to locate the change.
Source: Coding guidelines
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation