refactor: keep dependancies maintainable#14
Conversation
📝 WalkthroughWalkthroughThis PR performs a comprehensive code quality overhaul: it adds explicit return type annotations to ChangesComprehensive Type Annotations and Code Formatting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ingestion/local_file.py (1)
57-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace
logger.debug.This print statement violates the coding guideline. As per coding guidelines, use
logging, not♻️ Proposed fix
- print("Source ", source) + logger.debug(f"[LocalFile] Source: {source}")🤖 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 `@ingestion/local_file.py` at line 57, Replace the print("Source ", source) call with a logging call using the module logger (e.g., logger.debug("Source %s", source)) in the same place (keep the 'source' variable); if the module lacks a logger, add import logging and a module-level logger = logging.getLogger(__name__) so the debug call works and complies with the logging guideline.vector_store/qdrant_store.py (2)
76-138: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftRefactor
searchmethod to comply with function length guideline.The
searchmethod is 63 lines long, exceeding the 50-line limit specified in the coding guidelines. Consider extracting helper methods for filter construction and hit parsing to improve readability and testability.♻️ Suggested refactoring approach
Extract two helper methods:
def _build_search_filter(self, video_id: str | None) -> Filter | None: """Build Qdrant filter for video_id if provided.""" if not video_id: return None return Filter( must=[FieldCondition(key="video_id", match=MatchValue(value=video_id))] ) def _parse_search_hit(self, hit: Any) -> SearchResult | None: """Parse a Qdrant search hit into a SearchResult, or None if parsing fails.""" try: payload = hit.payload or {} chunk = VideoChunk( chunk_id=payload.get("chunk_id", str(hit.id)), video_id=payload.get("video_id", ""), text=payload.get("text", ""), start_time=float(payload.get("start_time", 0)), end_time=float(payload.get("end_time", 0)), segment_index=int(payload.get("segment_index", 0)), title=payload.get("title", ""), source_url=payload.get("source_url", ""), chapter=payload.get("chapter") or None, language=payload.get("language", "en"), ) return SearchResult(chunk=chunk, score=float(hit.score)) except Exception as hit_error: logger.warning(f"[Qdrant] Failed to parse hit id={hit.id}: {hit_error}") return NoneThen simplify
search:def search( self, query_vector: list[float], top_k: int = 5, filter_video_id: str | None = None, ) -> list[SearchResult]: try: query_filter = self._build_search_filter(filter_video_id) logger.debug(f"[Qdrant] Executing search with top_k={top_k}, filter={filter_video_id}") hits = self._client.search( collection_name=self._collection, query_vector=query_vector, limit=top_k, query_filter=query_filter, with_payload=True, ) results = [r for hit in hits if (r := self._parse_search_hit(hit)) is not None] except Exception as e: logger.error(f"[Qdrant] Search failed: {e!s}") logger.error(f"[Qdrant] Collection: {self._collection}, top_k: {top_k}, filter: {filter_video_id}") raise else: logger.debug(f"[Qdrant] Search completed successfully, found {len(results)} valid results") return resultsAs per coding guidelines: "Functions should be ≤ 50 lines, max 3 levels of nesting".
🤖 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 `@vector_store/qdrant_store.py` around lines 76 - 138, The search method is too long; extract two helpers to reduce its size: implement _build_search_filter(self, video_id: str | None) -> Filter | None to encapsulate the Filter/FieldCondition/MatchValue construction for filter_video_id, and implement _parse_search_hit(self, hit: Any) -> SearchResult | None to encapsulate the payload parsing into VideoChunk and SearchResult (catch parse exceptions and return None while logging the hit id). Replace the inline filter construction and hit parsing in search(...) with calls to these helpers, collect non-None results, and keep existing logging and exception behavior intact.
39-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove unjustified
cast(Any, ...)that defeats type checking.Casting
QdrantClienttoAnyremoves type safety and violates the coding guideline requiring full typing without unjustifiedAny. Theqdrant-clientlibrary provides proper type hints forQdrantClient. The cast can be safely removed:- self._client = cast(Any, QdrantClient(**client_kwargs)) + self._client = QdrantClient(**client_kwargs)The same issue applies to line 42 with
Embedder():- self._dim = cast(Any, Embedder()).dimension + self._dim = Embedder().dimension🤖 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 `@vector_store/qdrant_store.py` at line 39, Remove the unjustified cast to Any for the Qdrant client and embedder: replace the casted instantiation self._client = cast(Any, QdrantClient(**client_kwargs)) with a properly typed assignment using QdrantClient directly, and similarly remove cast(Any, Embedder()) for the embedder (e.g., self._embedder). Ensure the attributes are declared with appropriate types (e.g., self._client: QdrantClient and self._embedder: Embedder) so the module relies on the qdrant-client and Embedder type hints instead of casting to Any.pyproject.toml (1)
188-214:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove sqlalchemy from mypy overrides to let the plugin work.
The configuration includes the
sqlalchemy.ext.mypy.plugin(line 174) but also tells mypy toignore_missing_imports = trueforsqlalchemy.*(line 211). This is contradictory—the plugin provides proper type information for SQLAlchemy, so ignoring missing imports defeats its purpose and prevents you from catching type errors in SQLAlchemy usage.🔧 Proposed fix
[[tool.mypy.overrides]] module = [ "chromadb.*", "openai.*", "whisper.*", "sentence_transformers.*", "ffmpeg.*", "streamlit.*", "streamlit_extras.*", "langchain.*", "langchain_community.*", "langchain_openai.*", "langchain_anthropic.*", "langchain_ollama.*", "torch.*", "tiktoken.*", "uvicorn.*", "fastapi.*", "alembic.*", "loguru.*", "tqdm.*", "tenacity.*", "requests.*", - "sqlalchemy.*", "aiosqlite.*" ] ignore_missing_imports = true🤖 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 `@pyproject.toml` around lines 188 - 214, The mypy overrides currently list "sqlalchemy.*" under the ignore_missing_imports block which disables the sqlalchemy.ext.mypy.plugin; remove "sqlalchemy.*" from the module list in the [[tool.mypy.overrides]] section so SQLAlchemy's mypy plugin can provide types (keep the rest of the modules and ignore_missing_imports setting as-is). Ensure the sqlalchemy.ext.mypy.plugin remains enabled in the mypy plugin configuration so the plugin's type checks take effect.
🧹 Nitpick comments (9)
vector_store/qdrant_store.py (5)
130-132: 💤 Low valueConsider reverting logger calls to single-line format.
Both logger calls fit within the 120-character line length limit and would be more readable on single lines.
♻️ Proposed reformat to single lines
- logger.error( - f"[Qdrant] Collection: {self._collection}, top_k: {top_k}, filter: {filter_video_id}" - ) + logger.error(f"[Qdrant] Collection: {self._collection}, top_k: {top_k}, filter: {filter_video_id}")- logger.debug( - f"[Qdrant] Search completed successfully, found {len(results)} valid results" - ) + logger.debug(f"[Qdrant] Search completed successfully, found {len(results)} valid results")Also applies to: 135-137
🤖 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 `@vector_store/qdrant_store.py` around lines 130 - 132, The multi-line logger.error calls around the Qdrant query (using logger.error with f"[Qdrant] Collection: {self._collection}, top_k: {top_k}, filter: {filter_video_id}") should be reverted to single-line statements for readability; update both occurrences (the one shown and the similar one further down) to a single-line logger.error call each using the same f-string so they remain within the 120-char limit and match the project's style.
123-125: 💤 Low valueConsider reverting to single-line format.
This warning logger call is estimated at ~75 characters and fits within the 120-character line length limit.
♻️ Proposed reformat to single line
- logger.warning( - f"[Qdrant] Failed to parse hit id={hit.id}: {hit_error}" - ) + logger.warning(f"[Qdrant] Failed to parse hit id={hit.id}: {hit_error}")🤖 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 `@vector_store/qdrant_store.py` around lines 123 - 125, Revert the multi-line logger call in vector_store/qdrant_store.py back to a single-line warning: replace the current multi-line logger.warning(...) with a single-line f-string call that logs the hit id and hit_error (i.e., use logger.warning(f"[Qdrant] Failed to parse hit id={hit.id}: {hit_error}")), keeping the same message content and variables (logger.warning, hit.id, hit_error).
44-46: 💤 Low valueConsider reverting to single-line format.
This logger call is estimated at ~90 characters and fits comfortably within the Black 120-character line length limit. Splitting it across multiple lines reduces readability without providing a clear benefit.
♻️ Proposed reformat to single line
- logger.info( - f"[Qdrant] Connected — collection '{self._collection}', dim={self._dim}" - ) + logger.info(f"[Qdrant] Connected — collection '{self._collection}', dim={self._dim}")🤖 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 `@vector_store/qdrant_store.py` around lines 44 - 46, The logger.info call in qdrant_store.py is split across multiple lines; revert it to a single-line call so it reads like: logger.info(f"[Qdrant] Connected — collection '{self._collection}', dim={self._dim}"); update the call that references logger.info, self._collection and self._dim to be a single-line formatted string to improve readability and conform to the Black 120-char guideline.
93-95: 💤 Low valueConsider reverting to single-line format.
This debug logger call is estimated at ~90 characters and fits within the 120-character line length limit. The multiline split reduces readability without benefit.
♻️ Proposed reformat to single line
- logger.debug( - f"[Qdrant] Executing search with top_k={top_k}, filter={filter_video_id}" - ) + logger.debug(f"[Qdrant] Executing search with top_k={top_k}, filter={filter_video_id}")🤖 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 `@vector_store/qdrant_store.py` around lines 93 - 95, The logger.debug call was split across multiple lines reducing readability; revert the call to a single-line format by making the logger.debug invocation a single line that contains the f-string (logger.debug(f"[Qdrant] Executing search with top_k={top_k}, filter={filter_video_id}")), ensuring the message stays within line-length limits and no other changes to variables top_k or filter_video_id are made.
164-166: 💤 Low valueConsider keeping return statement on a single line.
The
returnstatement withcastis estimated at ~80 characters and fits within the 120-character limit. Splitting it reduces readability.♻️ Proposed reformat to single line
- return cast( - int, self._client.count(collection_name=self._collection, exact=True).count - ) + return cast(int, self._client.count(collection_name=self._collection, exact=True).count)🤖 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 `@vector_store/qdrant_store.py` around lines 164 - 166, The return statement in the method that returns the collection count should be collapsed to a single line for readability: replace the multi-line return using cast(int, ...) across lines with a single-line return that calls cast(int, self._client.count(collection_name=self._collection, exact=True).count). Locate the use of cast, self._client.count and self._collection in the method in qdrant_store.py and make this one-line change.ui/utils.py (3)
16-16: ⚡ Quick winGuideline violation and reduced type safety: Return type
Anyis too permissive.Both
api_getandapi_postreturnAny, but inspecting the code shows they returnhttpx.Response.json()(typicallydictorlist) orNoneon error. The currentAnyreturn type violates the coding guidelines and loses type information for callers.♻️ Recommended: Use specific return type union
-def api_get(path: str, **kwargs: Any) -> Any: +def api_get(path: str, **kwargs: Any) -> dict[str, Any] | list[Any] | None: try: r = httpx.get(f"{API_BASE}{path}", timeout=30, **kwargs) r.raise_for_status() return r.json() except httpx.HTTPStatusError as e: st.error(f"API error {e.response.status_code}: {e.response.text[:200]}") return None-def api_post(path: str, json: dict[str, object | None] | dict[str, str]) -> Any: +def api_post(path: str, json: dict[str, Any]) -> dict[str, Any] | list[Any] | None: try: r = httpx.post(f"{API_BASE}{path}", json=json, timeout=REQUEST_TIMEOUT) r.raise_for_status() return r.json() except httpx.HTTPStatusError as e: st.error(f"API error {e.response.status_code}: {e.response.text[:300]}") return NoneAs per coding guidelines: "All functions must be fully typed; no
Anywithout justification"Also applies to: 39-39
🤖 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 functions api_get and api_post currently declare return type Any; instead, tighten their annotations to reflect that they return either parsed JSON (dict or list) or None on error by changing their signatures (api_get and api_post) to return Optional[Union[Dict[str, Any], List[Any]]], and add the necessary typing imports (Optional, Union, Dict, List, Any) so callers get proper type information when they consume the httpx.Response.json() result or handle None.
81-81: ⚡ Quick winGuideline violation: Second tuple element uses
Anywithout justification.The return type
tuple[dict[str, str | None], Any]usesAnyfor the second element. Based on the implementation (line 83), it returnsvideos_datawhich is the result ofapi_getand should be typed consistently.♻️ Use consistent typing for API response
-def load_video_options() -> tuple[dict[str, str | None], Any]: +def load_video_options() -> tuple[dict[str, str | None], dict[str, Any] | None]: """Load available videos for selection dropdowns""" videos_data = api_get("/videos", params={"status": "indexed", "limit": 100})Or define a TypedDict for the videos API response:
class VideosResponseDict(TypedDict): videos: list[dict[str, Any]] # Add other known fields def load_video_options() -> tuple[dict[str, str | None], VideosResponseDict | None]:As per coding guidelines: "All functions must be fully typed; no
Anywithout justification"🤖 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 81, The return annotation for load_video_options is using Any for the second tuple element; replace it with a properly typed response type (or Optional of it) that matches videos_data returned by api_get: define a VideosResponseDict TypedDict (e.g., include videos: list[dict[str, Any]] and any other known fields) and update load_video_options signature to return tuple[dict[str, str | None], VideosResponseDict | None]; ensure you import TypedDict/Optional (and Any/List for inner types) and update any references to videos_data accordingly.
39-39: 💤 Low valueOverly complex type for
jsonparameter.The type
dict[str, object | None] | dict[str, str]is unnecessarily specific and restrictive. Sincehttpx.postaccepts any JSON-serializable dict, and the function passes it through without inspection,dict[str, Any]would be simpler and more flexible.♻️ Simplify json parameter type
-def api_post(path: str, json: dict[str, object | None] | dict[str, str]) -> Any: +def api_post(path: str, json: dict[str, Any]) -> Any:🤖 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 39, The api_post function's json parameter uses an overly complex union type; change its annotation to use a generic JSON-serializable mapping (e.g., dict[str, Any]) to match what httpx.post accepts. Update the signature of api_post to accept json: dict[str, Any] and ensure Any is imported from typing if not already; no runtime logic changes are required—just the type hint on api_post (and any related stub/type imports).ui/tabs/summarize_tab.py (1)
11-11: ⚡ Quick winGuideline violation:
Anyused without justification.The coding guidelines require "no
Anywithout justification." These functions usedict[str, Any]andlist[dict[str, Any]]extensively for API responses and video data structures. Consider defining TypedDict models for better type safety and IDE support.♻️ Recommended: Define TypedDict models for API structures
Add type definitions at the top of the file:
from typing import Any, TypedDict class VideoDict(TypedDict): id: str title: str status: str # Add other known fields class ChapterSummaryDict(TypedDict): chapter: str summary: str class SummaryResponseDict(TypedDict): title: str chunk_count: int overall_summary: str chapter_summaries: list[ChapterSummaryDict] | NoneThen update function signatures:
-def _get_video_choices(videos: list[dict[str, Any]]) -> dict[str, dict[str, Any]]: +def _get_video_choices(videos: list[VideoDict]) -> dict[str, VideoDict]: -def _render_summary_results(response: dict[str, Any]) -> None: +def _render_summary_results(response: SummaryResponseDict) -> None: -def _render_chapter_summaries(chapter_summaries: list[dict[str, Any]]) -> None: +def _render_chapter_summaries(chapter_summaries: list[ChapterSummaryDict]) -> None:As per coding guidelines: "All functions must be fully typed; no
Anywithout justification"Also applies to: 21-21, 32-32, 40-40, 45-45, 49-51, 54-54
🤖 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/tabs/summarize_tab.py` at line 11, The functions (e.g., _get_video_choices) currently use list[dict[str, Any]] and dict[str, Any]; replace these untyped Any usages by declaring concrete TypedDicts for the API shapes (e.g., VideoDict, ChapterSummaryDict, SummaryResponseDict) at the top of the file and then update function signatures and internal type hints to use those TypedDicts (e.g., list[VideoDict], SummaryResponseDict, list[ChapterSummaryDict] | None) so every function is fully typed and conforms to the "no Any without justification" guideline; ensure names match usages in _get_video_choices and the other affected functions so IDEs and static checkers pick up the types.
🤖 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 `@pyproject.toml`:
- Around line 216-222: The pytest config under [tool.pytest.ini_options]
references testpaths = ["tests"] but that directory is missing; either create a
top-level tests/ directory with at least one test file matching python_files =
["test_*.py"] (e.g., add tests/test_sample.py) so pytest finds tests, or update
the pyproject.toml testpaths value to the actual tests location (and ensure
python_files and python_functions patterns still match your test filenames);
modify the entry for testpaths in pyproject.toml accordingly and commit the new
directory or config change.
- Around line 22-74: The dependency versions in pyproject.toml allow vulnerable
releases; update the constraints for the affected packages: change
"torch>=2.2.0" to "torch>=2.6.0", "requests>=2.31.0" to "requests>=2.33.0",
"streamlit>=1.35.0" to "streamlit>=1.54.0", and "langchain>=0.2.0" to
"langchain>=0.2.5" (modify the lines referencing the symbols torch, requests,
streamlit, and langchain in the dependency list), then regenerate any lockfile /
reinstall to ensure the updated versions are resolved.
- Around line 158-177: The CI is failing because pyproject.toml enables overly
strict mypy settings; either fix all 27 type errors or relax the config so CI
can pass while you incrementally address them—specifically either (A) add
precise type annotations for FastAPI route handlers and retry-decorated
functions (references: any functions decorated with `@router.get`, `@router.post`,
`@router.delete` and `@retry`), fix functions in ingestion/, ui/, processing/, and
vector_store/ that currently return Any to return concrete types, correct DB
query assignments so variables typed as Video accept Video | None (reference:
Video usage), and add/vendor stubs or install type packages for langchain_core
and qdrant_client; or (B) temporarily disable strict flags that block CI by
removing "strict = true" and/or turning off disallow_untyped_decorators,
disallow_untyped_calls, and ignore_missing_imports for third‑party libs in
pyproject.toml (and keep show_error_codes/pretty), then gradually re-enable
checks as you address the listed issues. Ensure changes reference the symbols
above so reviewers can spot the fixes.
In `@transcription/whisper_transcriber.py`:
- Line 104: The _local_model field and _get_local_model return type currently
use Any; replace this with a proper Protocol that captures the minimal whisper
model interface (e.g., define a WhisperModelProtocol with a transcribe(...) ->
dict[str, Any] / dict containing "segments": list[...] and "language": str as
used in this file) and type _local_model: WhisperModelProtocol | None and update
_get_local_model to return WhisperModelProtocol; alternatively, if you prefer to
keep dynamic typing, add a type: ignore with a one-line justification on the
_local_model declaration and the _get_local_model signature stating
"openai-whisper has no type stubs/py.typed, so using type: ignore for this
external dynamic model." Ensure references to the model's transcribe call in
methods match the Protocol's method name and return shape.
In `@vector_store/qdrant_store.py`:
- Line 42: Remove the unjustified cast that defeats type checking: replace the
use of cast(Any, Embedder()).dimension with a properly typed access to
Embedder().dimension (e.g., assign self._dim from Embedder().dimension directly
and annotate self._dim with the correct type such as int). Update or ensure the
Embedder class in processing.embedder has a typed dimension attribute so the
type checker can verify it; modify the assignment in the class that defines
self._dim (the line using Embedder() and attribute dimension) and add an
explicit type annotation for self._dim if necessary.
---
Outside diff comments:
In `@ingestion/local_file.py`:
- Line 57: Replace the print("Source ", source) call with a logging call using
the module logger (e.g., logger.debug("Source %s", source)) in the same place
(keep the 'source' variable); if the module lacks a logger, add import logging
and a module-level logger = logging.getLogger(__name__) so the debug call works
and complies with the logging guideline.
In `@pyproject.toml`:
- Around line 188-214: The mypy overrides currently list "sqlalchemy.*" under
the ignore_missing_imports block which disables the sqlalchemy.ext.mypy.plugin;
remove "sqlalchemy.*" from the module list in the [[tool.mypy.overrides]]
section so SQLAlchemy's mypy plugin can provide types (keep the rest of the
modules and ignore_missing_imports setting as-is). Ensure the
sqlalchemy.ext.mypy.plugin remains enabled in the mypy plugin configuration so
the plugin's type checks take effect.
In `@vector_store/qdrant_store.py`:
- Around line 76-138: The search method is too long; extract two helpers to
reduce its size: implement _build_search_filter(self, video_id: str | None) ->
Filter | None to encapsulate the Filter/FieldCondition/MatchValue construction
for filter_video_id, and implement _parse_search_hit(self, hit: Any) ->
SearchResult | None to encapsulate the payload parsing into VideoChunk and
SearchResult (catch parse exceptions and return None while logging the hit id).
Replace the inline filter construction and hit parsing in search(...) with calls
to these helpers, collect non-None results, and keep existing logging and
exception behavior intact.
- Line 39: Remove the unjustified cast to Any for the Qdrant client and
embedder: replace the casted instantiation self._client = cast(Any,
QdrantClient(**client_kwargs)) with a properly typed assignment using
QdrantClient directly, and similarly remove cast(Any, Embedder()) for the
embedder (e.g., self._embedder). Ensure the attributes are declared with
appropriate types (e.g., self._client: QdrantClient and self._embedder:
Embedder) so the module relies on the qdrant-client and Embedder type hints
instead of casting to Any.
---
Nitpick comments:
In `@ui/tabs/summarize_tab.py`:
- Line 11: The functions (e.g., _get_video_choices) currently use list[dict[str,
Any]] and dict[str, Any]; replace these untyped Any usages by declaring concrete
TypedDicts for the API shapes (e.g., VideoDict, ChapterSummaryDict,
SummaryResponseDict) at the top of the file and then update function signatures
and internal type hints to use those TypedDicts (e.g., list[VideoDict],
SummaryResponseDict, list[ChapterSummaryDict] | None) so every function is fully
typed and conforms to the "no Any without justification" guideline; ensure names
match usages in _get_video_choices and the other affected functions so IDEs and
static checkers pick up the types.
In `@ui/utils.py`:
- Line 16: The functions api_get and api_post currently declare return type Any;
instead, tighten their annotations to reflect that they return either parsed
JSON (dict or list) or None on error by changing their signatures (api_get and
api_post) to return Optional[Union[Dict[str, Any], List[Any]]], and add the
necessary typing imports (Optional, Union, Dict, List, Any) so callers get
proper type information when they consume the httpx.Response.json() result or
handle None.
- Line 81: The return annotation for load_video_options is using Any for the
second tuple element; replace it with a properly typed response type (or
Optional of it) that matches videos_data returned by api_get: define a
VideosResponseDict TypedDict (e.g., include videos: list[dict[str, Any]] and any
other known fields) and update load_video_options signature to return
tuple[dict[str, str | None], VideosResponseDict | None]; ensure you import
TypedDict/Optional (and Any/List for inner types) and update any references to
videos_data accordingly.
- Line 39: The api_post function's json parameter uses an overly complex union
type; change its annotation to use a generic JSON-serializable mapping (e.g.,
dict[str, Any]) to match what httpx.post accepts. Update the signature of
api_post to accept json: dict[str, Any] and ensure Any is imported from typing
if not already; no runtime logic changes are required—just the type hint on
api_post (and any related stub/type imports).
In `@vector_store/qdrant_store.py`:
- Around line 130-132: The multi-line logger.error calls around the Qdrant query
(using logger.error with f"[Qdrant] Collection: {self._collection}, top_k:
{top_k}, filter: {filter_video_id}") should be reverted to single-line
statements for readability; update both occurrences (the one shown and the
similar one further down) to a single-line logger.error call each using the same
f-string so they remain within the 120-char limit and match the project's style.
- Around line 123-125: Revert the multi-line logger call in
vector_store/qdrant_store.py back to a single-line warning: replace the current
multi-line logger.warning(...) with a single-line f-string call that logs the
hit id and hit_error (i.e., use logger.warning(f"[Qdrant] Failed to parse hit
id={hit.id}: {hit_error}")), keeping the same message content and variables
(logger.warning, hit.id, hit_error).
- Around line 44-46: The logger.info call in qdrant_store.py is split across
multiple lines; revert it to a single-line call so it reads like:
logger.info(f"[Qdrant] Connected — collection '{self._collection}',
dim={self._dim}"); update the call that references logger.info, self._collection
and self._dim to be a single-line formatted string to improve readability and
conform to the Black 120-char guideline.
- Around line 93-95: The logger.debug call was split across multiple lines
reducing readability; revert the call to a single-line format by making the
logger.debug invocation a single line that contains the f-string
(logger.debug(f"[Qdrant] Executing search with top_k={top_k},
filter={filter_video_id}")), ensuring the message stays within line-length
limits and no other changes to variables top_k or filter_video_id are made.
- Around line 164-166: The return statement in the method that returns the
collection count should be collapsed to a single line for readability: replace
the multi-line return using cast(int, ...) across lines with a single-line
return that calls cast(int, self._client.count(collection_name=self._collection,
exact=True).count). Locate the use of cast, self._client.count and
self._collection in the method in qdrant_store.py and make this one-line change.
🪄 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: d0d93beb-4238-4a10-911c-dcedc637eefd
📒 Files selected for processing (29)
api/main.pyapi/models.pyapi/routes/ingest.pyapi/routes/query.pyconfig.pyingestion/base.pyingestion/live_stream.pyingestion/local_file.pyingestion/video_api.pyingestion/youtube.pyprocessing/chunker.pyprocessing/embedder.pypyproject.tomlrag/qa_chain.pyrag/retriever.pyrag/search.pyrag/summarizer.pystorage/database.pytranscription/whisper_transcriber.pyui/tabs/ingest_tab.pyui/tabs/library_tab.pyui/tabs/qa_tab.pyui/tabs/search_tab.pyui/tabs/summarize_tab.pyui/utils.pyvector_store/__init__.pyvector_store/base.pyvector_store/chroma_store.pyvector_store/qdrant_store.py
| "pydantic>=2.7.0", | ||
| "pydantic-settings>=2.2.0", | ||
| "python-dotenv>=1.0.1", | ||
|
|
||
| # Ingestion | ||
| "yt-dlp>=2024.1.1", | ||
| "ffmpeg-python>=0.2.0", | ||
| "requests>=2.31", | ||
| "requests>=2.31.0", | ||
|
|
||
| # Transcription | ||
| "openai-whisper>=20231117", | ||
|
|
||
| # LLM Providers | ||
| "openai>=1.30", | ||
| "anthropic>=0.25", | ||
| "langchain>=0.2", | ||
| "langchain-openai>=0.1", | ||
| "langchain-anthropic>=0.1", | ||
| "langchain-ollama>=0.1", | ||
| "langchain-community>=0.2", | ||
| "openai>=1.30.0", | ||
| "anthropic>=0.25.0", | ||
| "langchain>=0.2.0", | ||
| "langchain-openai>=0.1.0", | ||
| "langchain-anthropic>=0.1.0", | ||
| "langchain-ollama>=0.1.0", | ||
| "langchain-community>=0.2.0", | ||
|
|
||
| # Embeddings | ||
| "sentence-transformers>=2.7", | ||
| "torch>=2.0", | ||
| "sentence-transformers>=2.7.0", | ||
| "torch>=2.2.0", | ||
|
|
||
| # Vector Stores | ||
| "chromadb>=0.5", | ||
| "qdrant-client>=1.9", | ||
| "chromadb>=0.5.0", | ||
| "qdrant-client>=1.9.0", | ||
|
|
||
| # RAG | ||
| "tiktoken>=0.7", | ||
| "tiktoken>=0.7.0", | ||
| "nltk>=3.8.1", | ||
|
|
||
| # Storage | ||
| "sqlalchemy>=2.0", | ||
| "alembic>=1.13", | ||
| "aiosqlite>=0.20", | ||
| "sqlalchemy>=2.0.30", | ||
| "alembic>=1.13.1", | ||
| "aiosqlite>=0.20.0", | ||
|
|
||
| # API | ||
| "fastapi>=0.111", | ||
| "uvicorn[standard]>=0.29", | ||
| "fastapi>=0.111.0", | ||
| "uvicorn[standard]>=0.29.0", | ||
| "python-multipart>=0.0.9", | ||
| "httpx>=0.27", | ||
| "httpx>=0.27.0", | ||
|
|
||
| # UI | ||
| "streamlit>=1.35", | ||
| "streamlit-extras>=0.4", | ||
| "streamlit>=1.35.0", | ||
| "streamlit-extras>=0.4.0", | ||
|
|
||
| # Utilities | ||
| "tqdm>=4.66", | ||
| "loguru>=0.7", | ||
| "tenacity>=8.3", | ||
| "tqdm>=4.66.0", | ||
| "loguru>=0.7.2", | ||
| "tenacity>=8.3.0", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for security advisories on updated dependencies
echo "Checking security advisories for key dependencies..."
# Check key dependencies for known vulnerabilities
for package in "pydantic" "requests" "openai" "anthropic" "langchain" "torch" "chromadb" "fastapi" "streamlit"; do
echo "=== Checking $package ==="
gh api graphql -f query="
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: \"$package\") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}
" 2>/dev/null || echo "No advisories found or API error"
doneRepository: varit05/Streaming-Video-RAG
Length of output: 7541
Update dependency versions to address security vulnerabilities.
The version constraints as specified allow multiple packages with known security vulnerabilities:
- torch>=2.2.0: Contains CRITICAL remote code execution vulnerability via
torch.load(fixed in 2.6.0) and deserialization RCE (fixed in 2.3.1). Upgrade to torch>=2.6.0. - requests>=2.31.0: Vulnerable to insecure temp file reuse (fixed in 2.33.0), .netrc credentials leak (fixed in 2.32.4), and session verification bypass (fixed in 2.32.0). Upgrade to requests>=2.33.0.
- streamlit>=1.35.0: Vulnerable to SSRF on Windows (fixed in 1.54.0) and path traversal on Windows (fixed in 1.37.0). Upgrade to streamlit>=1.54.0.
- langchain>=0.2.0: Has denial of service vulnerability (fixed in 0.2.5). Upgrade to langchain>=0.2.5.
🤖 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 `@pyproject.toml` around lines 22 - 74, The dependency versions in
pyproject.toml allow vulnerable releases; update the constraints for the
affected packages: change "torch>=2.2.0" to "torch>=2.6.0", "requests>=2.31.0"
to "requests>=2.33.0", "streamlit>=1.35.0" to "streamlit>=1.54.0", and
"langchain>=0.2.0" to "langchain>=0.2.5" (modify the lines referencing the
symbols torch, requests, streamlit, and langchain in the dependency list), then
regenerate any lockfile / reinstall to ensure the updated versions are resolved.
| python_version = "3.12" | ||
| strict = true | ||
| ignore_missing_imports = false | ||
| disallow_untyped_defs = true | ||
| disallow_untyped_calls = true | ||
| disallow_incomplete_defs = true | ||
| disallow_subclassing_any = true | ||
| disallow_untyped_decorators = true | ||
| no_implicit_optional = true | ||
| warn_redundant_casts = false | ||
| warn_unused_ignores = false | ||
| warn_no_return = false | ||
| warn_return_any = false | ||
| strict_optional = false | ||
| warn_redundant_casts = true | ||
| warn_unused_ignores = true | ||
| warn_no_return = true | ||
| warn_return_any = true | ||
| strict_optional = true | ||
| plugins = [ | ||
| "pydantic.mypy", | ||
| "sqlalchemy.ext.mypy.plugin" | ||
| ] | ||
| show_error_codes = false | ||
| pretty = false | ||
| color_output = false | ||
| show_error_codes = true | ||
| pretty = true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Run mypy with the new strict configuration to check for type errors
echo "Running mypy with strict mode enabled..."
# Run mypy on all packages
mypy api ingestion llm processing rag storage transcription ui vector_store 2>&1 | head -100
echo ""
echo "=== Summary ==="
mypy api ingestion llm processing rag storage transcription ui vector_store 2>&1 | tail -20Repository: varit05/Streaming-Video-RAG
Length of output: 216
🏁 Script executed:
#!/bin/bash
# Install mypy and dependencies
pip install mypy pydantic sqlalchemy -q 2>&1
# Check if mypy was installed
which mypyRepository: varit05/Streaming-Video-RAG
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Look for CI/CD configuration files that run mypy
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.json" \) | grep -E "(github|gitlab|circleci|travis|codecov)" | head -20Repository: varit05/Streaming-Video-RAG
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Check project structure and identify package directories
ls -la | grep -E "^d" | grep -v "\."Repository: varit05/Streaming-Video-RAG
Length of output: 648
🏁 Script executed:
#!/bin/bash
# Look for type annotation coverage - check for stub files or type hints in main source
fd -e "pyi$" --type f | head -20Repository: varit05/Streaming-Video-RAG
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check CI workflow to see how mypy is run
cat ./.github/workflows/ci.ymlRepository: varit05/Streaming-Video-RAG
Length of output: 1440
🏁 Script executed:
#!/bin/bash
# Run mypy with PATH updated
export PATH="/home/jailuser/.local/bin:$PATH"
mypy api ingestion llm processing rag storage transcription ui vector_store --versionRepository: varit05/Streaming-Video-RAG
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Run mypy with PATH updated on the entire repo as per CI config
export PATH="/home/jailuser/.local/bin:$PATH"
mypy . 2>&1Repository: varit05/Streaming-Video-RAG
Length of output: 6147
Strict mypy configuration will cause CI failures — 27 type errors must be fixed before merge.
Running mypy with the strict configuration enabled reveals 27 errors across 15 files that prevent the build from passing:
- Untyped decorators (11 errors): FastAPI route decorators (
@router.get,@router.post,@router.delete) and@retrydecorators lack type information - Any return types (6 errors): Functions in
ingestion/,ui/,processing/, andvector_store/returnAnywhere specific types are declared - Missing library stubs (5 errors):
langchain_coreandqdrant_clientimports lack type stubs - Type assignment mismatches (3 errors): Database queries returning
Video | Noneassigned toVideovariables - Return type incompatibilities (2 errors): Tuple elements don't match declared types
The CI pipeline runs mypy . and will fail until these errors are resolved. Type annotations alone are insufficient—they must also satisfy strict mode constraints.
🤖 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 `@pyproject.toml` around lines 158 - 177, The CI is failing because
pyproject.toml enables overly strict mypy settings; either fix all 27 type
errors or relax the config so CI can pass while you incrementally address
them—specifically either (A) add precise type annotations for FastAPI route
handlers and retry-decorated functions (references: any functions decorated with
`@router.get`, `@router.post`, `@router.delete` and `@retry`), fix functions in
ingestion/, ui/, processing/, and vector_store/ that currently return Any to
return concrete types, correct DB query assignments so variables typed as Video
accept Video | None (reference: Video usage), and add/vendor stubs or install
type packages for langchain_core and qdrant_client; or (B) temporarily disable
strict flags that block CI by removing "strict = true" and/or turning off
disallow_untyped_decorators, disallow_untyped_calls, and ignore_missing_imports
for third‑party libs in pyproject.toml (and keep show_error_codes/pretty), then
gradually re-enable checks as you address the listed issues. Ensure changes
reference the symbols above so reviewers can spot the fixes.
| [tool.pytest.ini_options] | ||
| testpaths = ["tests"] | ||
| python_files = ["test_*.py"] | ||
| python_functions = ["test_*"] | ||
| python_classes = ["Test*"] | ||
| addopts = "-v --cov=api --cov=ingestion --cov=llm --cov=processing --cov=rag --cov=storage --cov=transcription --cov=ui --cov=vector_store --cov-report=term-missing" | ||
| asyncio_mode = "auto" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if tests directory exists and contains test files
echo "Checking for tests directory..."
if [ -d "tests" ]; then
echo "✓ tests/ directory exists"
echo ""
echo "Test files found:"
fd -t f 'test_.*\.py$' tests/ -x echo " {}"
echo ""
echo "Total test files: $(fd -t f 'test_.*\.py$' tests/ | wc -l)"
else
echo "✗ tests/ directory not found"
echo " testpaths configuration expects tests/ directory at repository root"
fiRepository: varit05/Streaming-Video-RAG
Length of output: 202
Create the tests/ directory or update the pytest configuration.
The pytest configuration specifies testpaths = ["tests"], but the tests/ directory does not exist at the repository root. Either create this directory structure and add test files matching the specified patterns (test_*.py), or update the testpaths configuration to point to where tests are actually located.
🤖 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 `@pyproject.toml` around lines 216 - 222, The pytest config under
[tool.pytest.ini_options] references testpaths = ["tests"] but that directory is
missing; either create a top-level tests/ directory with at least one test file
matching python_files = ["test_*.py"] (e.g., add tests/test_sample.py) so pytest
finds tests, or update the pyproject.toml testpaths value to the actual tests
location (and ensure python_files and python_functions patterns still match your
test filenames); modify the entry for testpaths in pyproject.toml accordingly
and commit the new directory or config change.
| def __init__(self): | ||
| def __init__(self) -> None: | ||
| self.mode = settings.whisper_mode | ||
| self._local_model: Any | None = None # lazy-loaded |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if whisper library has type information
python -c "import whisper; print(whisper.__file__)" 2>/dev/null || echo "whisper not installed"
# Search for any existing type handling for whisper in the codebase
rg -n "whisper" --type=py -C 2 | rg -i "protocol|typedalias|type.*ignore"Repository: varit05/Streaming-Video-RAG
Length of output: 93
🏁 Script executed:
# First, find and read the whisper_transcriber.py file
find . -name "whisper_transcriber.py" -type fRepository: varit05/Streaming-Video-RAG
Length of output: 110
🏁 Script executed:
# Read the whisper_transcriber.py file with line numbers
cat -n ./transcription/whisper_transcriber.py | head -200Repository: varit05/Streaming-Video-RAG
Length of output: 7710
🏁 Script executed:
# Let me check if whisper library has type stubs or py.typed marker
# First, let's see all imports and how whisper model is actually used
cat -n ./transcription/whisper_transcriber.py | sed -n '162,170p'Repository: varit05/Streaming-Video-RAG
Length of output: 536
🌐 Web query:
openai-whisper library type stubs py.typed typing support
💡 Result:
OpenAI Whisper (“openai-whisper” on PyPI) does not appear to ship separate PEP 561 type-stub packages (.pyi in a dedicated stubs distribution) or a py.typed marker file for typed package support. Evidence: - The upstream Whisper repository is shipped as the whisper Python module (setup config) without any packaging metadata that would indicate a PEP 561 typed package marker like py.typed in the source distribution: the project config sets py-modules = ["whisper"] and includes package data, but nothing in the shown packaging metadata references type-stub distribution conventions (e.g., package_data containing py.typed). [1][2] - The codebase includes normal Python type annotations in source files (for example whisper/model.py uses typing constructs such as Dict, Optional, Tuple), but that is not the same as providing PEP 561 “typed package” distribution support for static type checkers that rely on py.typed/stubs. [3] - Mypy’s documented behavior distinguishes between inline type information and stub-based/PEP 561-style support (stub packages require .pyi/type distribution mechanisms; py.typed is part of that ecosystem). [4] Practical takeaway: - You should expect partial type checking from the in-repo annotations, but for robust mypy/pyright support you likely need to add your own .pyi stubs (or install third-party stubs if they exist) and/or configure MYPYPATH/stub paths, because the upstream distribution does not clearly advertise a PEP 561 typed package or dedicated type stubs. [4][5] If you want, tell me whether you use mypy or pyright, and I can draft a minimal stubs/whisper/init.pyi (and possibly for transcribe.py) that matches the actual runtime API signatures described in the repo (e.g., whisper.load_model(...), model.transcribe(...)).
Citations:
- 1: https://github.com/openai/whisper/blob/ba3f3cd54b0e5b8ce1ab3de13e32122d0d5f98ab/setup.py
- 2: https://github.com/openai/whisper/blob/main/pyproject.toml
- 3: https://github.com/openai/whisper/blob/main/whisper/model.py
- 4: https://mypy.readthedocs.io/en/stable/stubs.html
- 5: https://stackoverflow.com/questions/65947076/why-is-mypy-finding-no-type-hints-or-library-stubs-for-any-of-my-imports
Define a Protocol for the Whisper model type or use type: ignore with justification.
The _local_model field (line 104) and _get_local_model return type (line 162) use Any with only a "lazy-loaded" comment. Per coding guidelines, "All functions must be fully typed; no Any without justification." The justification must address the type choice, not the design pattern.
The openai-whisper library lacks PEP 561 type stubs or a py.typed marker, so Any cannot be justified by available type information. To comply with the guideline, either:
- Define a Protocol that documents the expected whisper model interface (recommended, improves maintainability)
- Use
type: ignorewith a comment explaining that whisper lacks type stubs
The Protocol approach, defining the interface based on actual usage (the transcribe method that returns a dict with "segments" and "language" keys), would be clearer than bare Any.
Protocol-based solution
+from typing import Protocol
+
+class WhisperModel(Protocol):
+ """Protocol for whisper model interface."""
+ def transcribe(self, audio: str, **kwargs) -> dict[str, Any]: ...
+
class WhisperTranscriber:
"""
Transcribes audio files using Whisper.
Mode is determined by settings.whisper_mode.
"""
def __init__(self) -> None:
self.mode = settings.whisper_mode
- self._local_model: Any | None = None # lazy-loaded
+ self._local_model: WhisperModel | None = None
- def _get_local_model(self) -> Any:
+ def _get_local_model(self) -> WhisperModel:
"""Lazy-load the Whisper model (avoids loading it until needed)."""🤖 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 `@transcription/whisper_transcriber.py` at line 104, The _local_model field and
_get_local_model return type currently use Any; replace this with a proper
Protocol that captures the minimal whisper model interface (e.g., define a
WhisperModelProtocol with a transcribe(...) -> dict[str, Any] / dict containing
"segments": list[...] and "language": str as used in this file) and type
_local_model: WhisperModelProtocol | None and update _get_local_model to return
WhisperModelProtocol; alternatively, if you prefer to keep dynamic typing, add a
type: ignore with a one-line justification on the _local_model declaration and
the _get_local_model signature stating "openai-whisper has no type
stubs/py.typed, so using type: ignore for this external dynamic model." Ensure
references to the model's transcribe call in methods match the Protocol's method
name and return shape.
| @@ -41,7 +41,9 @@ def __init__(self) -> None: | |||
| # Infer dimension from embedder | |||
| self._dim = cast(Any, Embedder()).dimension | |||
There was a problem hiding this comment.
Remove unjustified cast(Any, ...) that defeats type checking.
Casting Embedder() to Any removes type safety and violates the coding guideline. Since Embedder is a local class from processing.embedder, it should have proper type hints for the dimension attribute.
🔒 Proposed fix to preserve type information
- self._dim = cast(Any, Embedder()).dimension
+ self._dim = Embedder().dimensionAs per coding guidelines: "All functions must be fully typed; no Any without justification".
📝 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.
| self._dim = cast(Any, Embedder()).dimension | |
| self._dim = Embedder().dimension |
🤖 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 `@vector_store/qdrant_store.py` at line 42, Remove the unjustified cast that
defeats type checking: replace the use of cast(Any, Embedder()).dimension with a
properly typed access to Embedder().dimension (e.g., assign self._dim from
Embedder().dimension directly and annotate self._dim with the correct type such
as int). Update or ensure the Embedder class in processing.embedder has a typed
dimension attribute so the type checker can verify it; modify the assignment in
the class that defines self._dim (the line using Embedder() and attribute
dimension) and add an explicit type annotation for self._dim if necessary.
Summary by CodeRabbit
Release Notes