Feat/add ruff linting - #209
Conversation
📝 WalkthroughWalkthroughModernized type hints across the codebase to Python 3.10+ syntax (built-in generics and | unions), added Ruff linting config and CI workflow, and applied formatting/exception-message consistency changes across many modules; also added minor runtime wiring (Reranker client) and evaluation pipeline enhancements. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
automatic-evaluation-pipeline/generate_questions.py (2)
154-158: Missing null check before unpackingall_chunks_list.
get_all_chunks()returnsNoneon failure (line 126), but the code proceeds to unpackall_chunks_listwithout validation. This will cause aTypeErrorwhen iterating overNone.Proposed fix
all_chunks_list = await get_all_chunks(url) + if not all_chunks_list: + logger.error("Failed to retrieve chunks. Exiting.") + return pause = time.time()
179-186: HDBSCAN computation is dead code.Lines 180-181 compute HDBSCAN labels, but lines 184-186 immediately overwrite the
labelsvariable with DBSCAN results. The HDBSCAN computation is wasted.Either remove the unused HDBSCAN code, or make the clustering algorithm selectable via configuration.
Option 1: Remove dead HDBSCAN code
- # Here you have the choice up clustering using HDBSCAN - hdb = hdbscan.HDBSCAN(min_cluster_size=5, metric="euclidean") - labels = hdb.fit_predict(embeddings) - - # or dbscan + # Clustering using DBSCAN db = DBSCAN(eps=0.1, min_samples=3, metric="cosine") db_labels = db.fit(embeddings) labels = db_labels.labels_This would also allow removing the
hdbscanimport on line 8.openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
71-77: Bug:configshould beself.configon line 77.Line 76 creates
self.configvia a localload_config()call, but line 77 references the module-levelconfigvariable instead. This inconsistency makes the instance config partially unused and could cause subtle issues if configurations differ.🐛 Proposed fix
def __init__(self): from config import load_config from utils.logger import get_logger self.logger = get_logger() self.config = load_config() - self.pool_size = config.loader.get("docling_pool_size", 1) + self.pool_size = self.config.loader.get("docling_pool_size", 1)openrag/components/reranker.py (1)
37-40: Add timeout to external rerank API call.The async call to the external rerank service has no timeout. If the service is slow or unresponsive, this will block indefinitely, potentially exhausting the semaphore slots and causing request pile-up.
Consider wrapping the call with
asyncio.wait_for():Proposed fix
try: - rerank_result: ReRankResult = await rerank.asyncio( - client=self.client, body=rerank_input + rerank_result: ReRankResult = await asyncio.wait_for( + rerank.asyncio(client=self.client, body=rerank_input), + timeout=30.0, # Configure as needed )automatic-evaluation-pipeline/benchmark.py (1)
294-306: Logic bug: nDCG scores may misalign with valid scores when errors occur mid-list.The current slicing
nDCG_scores[:len(valid_scores)]assumes errors only occur at the end. If an error occurs at any other position, the nDCG scores won't correspond to the correct questions.For example, if question 2 fails:
llm_judge_scores = [score1, "error", score3, score4, score5]valid_scores = [score1, score3, score4, score5]valid_ndcg_scores = nDCG_scores[:4] = [ndcg1, ndcg2, ndcg3, ndcg4]This incorrectly pairs
score3withndcg2.🐛 Proposed fix: Filter nDCG scores alongside judge scores
- # Filter out error responses - valid_scores = [(comp, prec) for comp, prec in llm_judge_scores if comp != "error"] - valid_ndcg_scores = nDCG_scores[: len(valid_scores)] # Match the filtered scores + # Filter out error responses and keep corresponding nDCG scores aligned + valid_scores = [] + valid_ndcg_scores = [] + for (comp, prec), ndcg in zip(llm_judge_scores, nDCG_scores): + if comp != "error": + valid_scores.append((comp, prec)) + valid_ndcg_scores.append(ndcg)openrag/routers/tools.py (1)
54-58: Bug: Generator expression is always truthy, validation never fails.The condition
if not (t.name == name for t in AVAILABLE_TOOLS)creates a generator object, which is always truthy regardless of what it yields. This means the tool name validation is effectively bypassed — any tool name will pass this check.Use
any()to properly evaluate the generator:🐛 Proposed fix
- if not (t.name == name for t in AVAILABLE_TOOLS): + if not any(t.name == name for t in AVAILABLE_TOOLS):openrag/api.py (1)
211-220: Duplicateopenai_routerinclusion when Chainlit UI is enabled.When
WITH_OPENAI_APIisTrueANDWITH_CHAINLIT_UIisTrue, theopenai_routerwill be included twice (lines 213 and 220), which will cause route conflicts or duplicate endpoint registrations.Proposed fix
if WITH_CHAINLIT_UI: # Mount the default front from chainlit.utils import mount_chainlit mount_chainlit(app, "./app_front.py", path="/chainlit") - app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) # cause chainlit uses openai api endpoints + # Include openai_router only if not already included + if not WITH_OPENAI_API: + app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) # cause chainlit uses openai api endpointsopenrag/components/indexer/loaders/serializer.py (1)
39-50: Mutable default argument andNonehandling issue.Two issues here:
Mutable default argument: Using
= {}as a default value is a Python antipattern because the same dict instance is shared across all calls.Type/default mismatch: The type annotation
dict | NonesuggestsNoneis a valid argument, but if a caller explicitly passesNone, line 47'smetadata.get("file_id")will raiseAttributeError: 'NoneType' object has no attribute 'get'.Other loaders in this PR use
= Nonewith an explicit guard, which is the safer pattern.🐛 Proposed fix
async def serialize_document( self, task_id: str, path: str | Path, - metadata: dict | None = {}, + metadata: dict | None = None, ) -> Document: + if metadata is None: + metadata = {} # Set task state log = self.logger.bind(openrag/components/indexer/chunker/chunker.py (1)
112-132: PotentialTypeErrorwhenllm_configisNone.The parameter
llm_configacceptsNoneas a default, but line 124 callsChatOpenAI(**llm_config)which will raiseTypeError: 'NoneType' object is not a mappingifllm_configisNone.🐛 Proposed fix - add guard or change default
Option 1: Add a guard before using:
def __init__( self, chunk_size: int = 200, chunk_overlap_rate: float = 0.2, llm_config: dict | None = None, contextual_retrieval: bool = False, **kwargs, ): self.chunk_size = chunk_size self.chunk_overlap_rate = chunk_overlap_rate self.chunk_overlap = int(self.chunk_size * self.chunk_overlap_rate) + if llm_config is None: + raise ValueError("llm_config is required") self.llm = ChatOpenAI(**llm_config)Option 2: Change signature to require llm_config (remove None default):
def __init__( self, chunk_size: int = 200, chunk_overlap_rate: float = 0.2, - llm_config: dict | None = None, + llm_config: dict, contextual_retrieval: bool = False, **kwargs, ):openrag/components/indexer/indexer.py (1)
219-236: Mutable default argumentfilter: dict | None = {}.Same issue as above - using
{}as a default value shares the same dictionary instance across calls.🐛 Proposed fix
`@ray.method`(concurrency_group="search") async def asearch( self, query: str, top_k: int = 5, similarity_threshold: float = 0.80, partition: str | list[str] | None = None, - filter: dict | None = {}, + filter: dict | None = None, ) -> list[Document]: partition_list = self._check_partition_list(partition) vectordb = ray.get_actor("Vectordb", namespace="openrag") + if filter is None: + filter = {} return await vectordb.async_search.remote(
🤖 Fix all issues with AI agents
In @.github/workflows/lint.yml:
- Around line 26-29: Replace the manual curl installer block that adds uv (the
"Install uv" step) with the official astral-sh/setup-uv action (use
astral-sh/setup-uv@v7) to pin the version and enable automatic caching; remove
the manual $HOME/.local/bin append and any separate caching steps since setup-uv
manages them, and pass a pinned version via the action's "version" input to
ensure reproducible installs.
In `@LINTING.md`:
- Line 107: Fix the typo in the LINTING.md text by replacing the misspelled word
"unecessary" with the correct spelling "unnecessary" in the sentence "Sometimes,
you will have to ignore a particular check if deemed unecessary".
- Around line 88-95: The pre-commit hook uses an incorrect command and
auto-stages all files; replace the line "uv run git add ." with a plain git
invocation that only updates tracked files (e.g., "git add -u") or remove the
auto-staging entirely so developers control what gets committed; update the hook
script block where the commands are defined (the here-doc creating
.git/hooks/pre-commit) to call "git add -u" (or delete that line) and ensure the
other commands remain "uv run ruff check . --fix" and "uv run ruff format .",
then keep the chmod +x step.
In `@openrag/api.py`:
- Around line 68-72: The type hints for WITH_CHAINLIT_UI and WITH_OPENAI_API are
incorrect: change their annotations from "bool | None" to "bool" because the
expressions os.getenv(..., "true").lower() == "true" always produce a boolean;
update the variable declarations (WITH_CHAINLIT_UI and WITH_OPENAI_API) to use
type hint bool and leave the runtime expression as-is.
In `@openrag/components/indexer/indexer.py`:
- Around line 58-64: The add_file function uses a mutable default for metadata
(metadata: dict | None = {}), which can be shared across calls; change the
signature to use None as the default (metadata: dict | None = None or
Optional[dict]) and inside add_file initialize a fresh dict when metadata is
None (e.g., metadata = {} if metadata is None) before any modifications; update
any type hints or callers if needed and ensure the same pattern for other
mutable defaults like path if applicable.
In `@openrag/components/indexer/loaders/base.py`:
- Around line 37-43: The abstract async method aload_document is missing the
instance parameter; update its signature to include self as the first parameter
(async def aload_document(self, file_path: str | Path, metadata: dict | None =
None, save_markdown: bool = False):) and ensure any subclasses implement the
matching signature so the abstractmethod contract is satisfied.
In `@openrag/components/indexer/loaders/eml_loader.py`:
- Line 30: The type annotation for function aload_document is inconsistent
because it declares metadata: dict = None; update the signature to use a proper
PEP 604 union so the default None matches the type—e.g., change metadata to dict
| None = None (or Optional[dict] if using typing) while leaving save_markdown:
bool = False unchanged; update any related type hints or callers if needed to
reflect the new annotation for the aload_document method.
- Around line 232-238: The call to self.get_image_description uses the wrong
keyword argument name; change the call in eml_loader.py (where image =
Image.open... and caption = await self.get_image_description(image=image)) to
use the expected parameter name image_data (i.e., caption = await
self.get_image_description(image_data=image)) so it matches the base loader's
get_image_description signature.
- Around line 193-194: The call uses the wrong keyword for
get_image_description; change the invocation to pass the image under the correct
parameter name (e.g., await self.get_image_description(image_data=image)) or
pass the image positionally (await self.get_image_description(image)) so the
base method parameter image_data matches and avoids the TypeError; update the
call near where Image.open(...) is assigned before caption to use image_data.
- Around line 204-206: The condition referencing fallback_success can raise
NameError for non-PDF files; initialize fallback_success to False before any
loader attempts (e.g., at the start of the relevant function or before the PDF
handling block) or change the conditional in the branch that contains 'elif
file_ext in [".txt", ".docx", ".doc"] or (file_ext == ".pdf" and not
fallback_success)' so that fallback_success is only evaluated when file_ext ==
".pdf" (for example, split into two separate conditions), ensuring
fallback_success is always defined before use and avoiding referencing it for
non-PDF extensions.
In `@openrag/components/indexer/loaders/pdf_loaders/openai.py`:
- Around line 18-20: The module-level function pdf_to_images incorrectly
declares a self parameter; remove self from its signature so it becomes async
def pdf_to_images(pdf_path: str, scale: float = 1.0) -> list[Image.Image],
adjust the body accordingly (no use of self), and update any call sites that
currently pass an instance as the first argument to instead call
pdf_to_images(pdf_path, scale) so arguments line up correctly.
In `@openrag/components/indexer/utils/files.py`:
- Line 70: The function serialize_file uses a mutable default for metadata
(metadata: dict | None = {}), which can cause shared-state bugs; change the
signature to default metadata to None (e.g., metadata: dict | None = None) and
inside the function initialize a fresh dict when needed (e.g., if metadata is
None: metadata = {}), ensuring serialize_file and any callers (by name) no
longer rely on a shared mutable default.
In `@openrag/models/openai.py`:
- Around line 55-59: The ChoiceLogprobs model has a type mismatch: the tokens
field is annotated as list[str] but given a default of None; update the tokens
declaration in class ChoiceLogprobs so its type matches the default (make it
list[str] | None) or provide a non-None default consistent with list[str];
adjust the tokens field signature to be tokens: list[str] | None = Field(None)
(or set tokens: list[str] = Field(default_factory=list)) so Pydantic validation
and typing are consistent.
In `@openrag/routers/indexer.py`:
- Line 282: The put_file endpoint is calling indexer.add_file.remote without
passing the current user, but add_file (in
openrag/components/indexer/indexer.py) accepts user: dict | None; update the
call in routers/indexer.py (the put_file handler) to include user=user (e.g.,
task = indexer.add_file.remote(path=file_path, metadata=metadata,
partition=partition, user=user)); ensure the handler uses the same user variable
used by the add_file endpoint (or obtains it from the request/auth dependency)
so the user dict is in scope when invoking indexer.add_file.remote.
In `@openrag/scripts/restore.py`:
- Around line 268-270: The --user-id argument is defined without type=int
causing args.user_id to be a string; update the parser.add_argument call for
"-u/--user-id" to include type=int so args.user_id is an integer before it's
passed to add_file_to_partition (and any other functions expecting an int),
ensuring downstream calls like add_file_to_partition receive the correct type.
🧹 Nitpick comments (12)
utility/data_indexer.py (1)
65-65: Consider removing or converting this debug print statement.This appears to be leftover debug code. Since the rest of the file uses
loggerfor output, consider either removing this line or converting it to a logger call for consistency.Suggested fix
-print(dir_path.is_dir()) +logger.debug(f"Directory check: {dir_path.is_dir()}")Or simply remove if not needed for debugging.
openrag/components/indexer/utils/test_text_sanitizer.py (1)
143-148: Minor inconsistency with test_chunking.py.This formatting change is fine, but note that
test_chunking.py(lines 183-190) has the sametest_multiline_spacingtest with a multi-line assertion format. Consider aligning the assertion styles across both test files for consistency, or consolidating the duplicate test.openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
92-98: Type annotation is misleading for Ray actor handle.
actoris actually aray.actor.ActorHandle, not aDoclingWorkerinstance. The type hint is functionally harmless but could confuse readers or cause type-checker warnings.♻️ Suggested fix
async def process_pdf(self, file_path: str) -> ConversionResult: - actor: DoclingWorker = await self._queue.get() + actor: ray.actor.ActorHandle = await self._queue.get()openrag/components/reranker.py (2)
41-46: Mutating input documents may cause unintended side effects.The method modifies the
metadataof the inputdocumentsdirectly. Callers may not expect their original documents to be altered. Consider creating shallow copies to avoid side effects:Proposed fix
output = [] for rerank_res in rerank_result.results: - doc = documents[rerank_res.index] - doc.metadata["relevance_score"] = rerank_res.relevance_score + original_doc = documents[rerank_res.index] + doc = Document( + page_content=original_doc.page_content, + metadata={**original_doc.metadata, "relevance_score": rerank_res.relevance_score}, + ) output.append(doc)
48-55: Use bareraiseto preserve full traceback.Using
raise einstead ofraisecan subtly alter the traceback in some edge cases. The idiomatic Python pattern is to use bareraise:Proposed fix
except Exception as e: self.logger.error( "Reranking failed", error=str(e), model_name=self.model_name, documents_count=len(documents), ) - raise e + raiseautomatic-evaluation-pipeline/benchmark.py (2)
58-70: Consider simplifying the control flow and removing the external link.
- The ChatGPT link in the comment (line 59) is not ideal - external links can break and don't provide context for future maintainers.
- The
keyboolean flag adds unnecessary complexity when you can return directly.♻️ Suggested simplification
def compute_inverted_ranks(true_chunk_id, all_retrieved_chunks): - # see link: https://chatgpt.com/share/6813f998-2e88-8002-a472-6af2e9a64b61 - key = False try: rank = all_retrieved_chunks.index(true_chunk_id) + 1 - key = True + return 1 / rank except ValueError: logger.debug(f"ValueError: {true_chunk_id} not found in retrieved_ids") - - if key: - return 1 / rank - else: return 0
227-230: Consider makingpartitionconfigurable via environment variable.Other configuration values (
APP_PORT,APP_URL) are sourced from environment variables, butpartitionis hardcoded. This inconsistency could lead to errors when running benchmarks against different partitions.♻️ Suggested change
num_port = os.environ.get("APP_PORT") num_host = os.environ["APP_URL"] openrag_api_base_url = f"http://{num_host}:{num_port}" - partition = "pdftest" # To replace with your wanted partition's name + partition = os.environ.get("PARTITION", "pdftest")openrag/components/pipeline.py (1)
206-208: LGTM on the formatting change.The
{e!s}conversion is idiomatic and equivalent tostr(e).Minor pre-existing nit: the log message says "chat completion" but this is in the
completionsmethod. Consider aligning the message with the method name for clarity.openrag/components/prompts/prompts.py (1)
25-27: Consider specifying explicit encoding for cross-platform consistency.While removing the redundant
"r"mode is fine, consider adding an explicitencoding="utf-8"parameter. This ensures consistent behavior across different platforms and locales, and is a common best practice (PEP 597).♻️ Suggested improvement
- with open(file_path) as f: + with open(file_path, encoding="utf-8") as f: sys_msg = f.read() return sys_msgopenrag/scripts/embed.py (1)
246-272: Consider extracting duplicateopen_output_filefunction to a shared utility.This function is duplicated in
openrag/scripts/backup.py(lines 139-167). Consider extracting it to a shared utility module to avoid code duplication and ensure consistency.openrag/api.py (1)
35-41:ray.init()at module import time may cause side effects.Calling
ray.init()at module level means Ray initializes whenever this module is imported, including during tests or when importing for introspection. This can cause unexpected behavior or resource allocation.Consider deferring initialization to an explicit startup point or guarding it.
Suggested approach
-ray.init(dashboard_host="0.0.0.0") +# Defer ray.init to avoid side effects on import +if not ray.is_initialized(): + ray.init(dashboard_host="0.0.0.0")Or move initialization inside the
if __name__ == "__main__":block.tests/api_tests/test_indexer.py (1)
73-99: Fixed sleep delay may cause test flakiness.The
time.sleep(2)is a hardcoded delay that may not be sufficient in slow CI environments, or may unnecessarily slow down tests in fast environments. Consider polling for the upload status instead of a fixed sleep.Alternative approach using polling
# Poll for task completion instead of fixed sleep for _ in range(10): # Max 10 attempts time.sleep(0.5) # Check if file is registered (e.g., via list files endpoint) status_resp = api_client.get(f"/partition/{created_partition}/files") if status_resp.status_code == 200 and file_id in str(status_resp.json()): breakHowever, if the current 2-second sleep is sufficient for your CI environment and test reliability is acceptable, this can be deferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
.github/workflows/api_tests/mock_vllm.py.github/workflows/lint.ymlLINTING.mdautomatic-evaluation-pipeline/benchmark.pyautomatic-evaluation-pipeline/generate_questions.pyautomatic-evaluation-pipeline/upload_files.pyopenrag/api.pyopenrag/app_front.pyopenrag/chainlit_api.pyopenrag/components/indexer/chunker/chunker.pyopenrag/components/indexer/chunker/utils.pyopenrag/components/indexer/embeddings/__init__.pyopenrag/components/indexer/embeddings/openai.pyopenrag/components/indexer/indexer.pyopenrag/components/indexer/loaders/__init__.pyopenrag/components/indexer/loaders/base.pyopenrag/components/indexer/loaders/eml_loader.pyopenrag/components/indexer/loaders/image.pyopenrag/components/indexer/loaders/pdf_loaders/docling2.pyopenrag/components/indexer/loaders/pdf_loaders/marker.pyopenrag/components/indexer/loaders/pdf_loaders/openai.pyopenrag/components/indexer/loaders/serializer.pyopenrag/components/indexer/loaders/test_media_loader.pyopenrag/components/indexer/loaders/txt_loader.pyopenrag/components/indexer/utils/files.pyopenrag/components/indexer/utils/test_text_sanitizer.pyopenrag/components/indexer/vectordb/utils.pyopenrag/components/indexer/vectordb/vectordb.pyopenrag/components/llm.pyopenrag/components/map_reduce.pyopenrag/components/pipeline.pyopenrag/components/prompts/prompts.pyopenrag/components/reranker.pyopenrag/components/retriever.pyopenrag/components/utils.pyopenrag/models/indexer.pyopenrag/models/openai.pyopenrag/routers/actors.pyopenrag/routers/extract.pyopenrag/routers/indexer.pyopenrag/routers/openai.pyopenrag/routers/partition.pyopenrag/routers/queue.pyopenrag/routers/search.pyopenrag/routers/tools.pyopenrag/routers/users.pyopenrag/routers/utils.pyopenrag/scripts/backup.pyopenrag/scripts/embed.pyopenrag/scripts/migrations/alembic/versions/4add4d260575_initial_migration.pyopenrag/scripts/migrations/alembic/versions/cd642e4502d8_create_users_memberships_tables.pyopenrag/scripts/restore.pyopenrag/utils/dependencies.pypyproject.tomltests/api_tests/conftest.pytests/api_tests/test_actors.pytests/api_tests/test_extract.pytests/api_tests/test_indexer.pytests/api_tests/test_openai_compat.pytests/api_tests/test_partition.pytests/api_tests/test_queue.pytests/api_tests/test_search.pytests/api_tests/test_tools.pytests/api_tests/test_users.pytests/test_vectordb.pyutility/data_indexer.py
💤 Files with no reviewable changes (6)
- tests/api_tests/test_users.py
- tests/api_tests/test_queue.py
- tests/api_tests/test_tools.py
- tests/api_tests/test_partition.py
- tests/api_tests/test_extract.py
- tests/api_tests/test_actors.py
🧰 Additional context used
🧬 Code graph analysis (17)
openrag/components/indexer/loaders/image.py (1)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(18-152)
openrag/components/indexer/vectordb/utils.py (1)
openrag/components/indexer/vectordb/vectordb.py (3)
list_partition_files(57-58)list_partition_files(718-735)get_user_by_token(887-888)
openrag/components/map_reduce.py (1)
openrag/components/indexer/indexer.py (1)
chunk(52-56)
openrag/components/indexer/loaders/__init__.py (1)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(18-152)
openrag/components/indexer/utils/test_text_sanitizer.py (2)
openrag/components/indexer/chunker/test_chunking.py (1)
test_multiline_spacing(184-191)openrag/components/indexer/utils/text_sanitizer.py (1)
clean_markdown_table_spacing(98-127)
openrag/scripts/embed.py (1)
openrag/scripts/backup.py (1)
open_output_file(140-168)
openrag/routers/users.py (2)
openrag/components/indexer/vectordb/utils.py (1)
delete_user(424-431)openrag/components/indexer/vectordb/vectordb.py (1)
delete_user(875-882)
openrag/routers/openai.py (1)
openrag/models/openai.py (2)
OpenAIChatCompletionRequest(14-30)OpenAICompletionRequest(75-92)
openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
openrag/config/config.py (1)
load_config(12-29)
openrag/api.py (4)
openrag/config/config.py (1)
load_config(12-29)openrag/utils/dependencies.py (1)
get_vectordb(47-49)openrag/utils/exceptions/base.py (1)
OpenRAGError(4-24)openrag/utils/logger.py (1)
get_logger(10-50)
automatic-evaluation-pipeline/generate_questions.py (1)
openrag/components/indexer/indexer.py (1)
chunk(52-56)
openrag/scripts/backup.py (3)
openrag/components/indexer/vectordb/utils.py (2)
list_partition_files(181-204)list_partitions(295-299)openrag/utils/logger.py (1)
get_logger(10-50)openrag/scripts/embed.py (1)
open_output_file(246-272)
tests/api_tests/test_indexer.py (1)
tests/api_tests/conftest.py (4)
api_client(16-19)created_partition(87-98)sample_markdown_file(57-77)sample_text_file(39-53)
openrag/components/indexer/indexer.py (2)
openrag/components/indexer/chunker/chunker.py (1)
split_document(259-285)openrag/routers/indexer.py (1)
add_file(115-172)
openrag/components/indexer/loaders/eml_loader.py (4)
openrag/components/indexer/loaders/__init__.py (1)
get_loader_classes(18-51)openrag/components/indexer/loaders/base.py (3)
BaseLoader(18-152)aload_document(38-43)get_image_description(66-152)openrag/components/indexer/loaders/image.py (1)
aload_document(13-20)openrag/components/indexer/loaders/pdf_loaders/openai.py (1)
aload_document(44-71)
openrag/routers/indexer.py (2)
openrag/components/indexer/indexer.py (3)
add_file(58-131)set_state(281-284)copy_file(186-217)openrag/routers/utils.py (2)
validate_metadata(200-208)require_task_owner(156-170)
openrag/components/indexer/chunker/chunker.py (1)
openrag/components/indexer/chunker/utils.py (3)
MDElement(20-34)split_md_elements(56-110)get_chunk_page_number(113-148)
🪛 LanguageTool
LINTING.md
[uncategorized] ~68-~68: The official name of this software platform is spelled with a capital “H”.
Context: ...Integration A GitHub Actions workflow (.github/workflows/lint.yml) automatically runs...
(GITHUB)
[grammar] ~107-~107: Ensure spelling is correct
Context: ... to ignore a particular check if deemed unecessary python # Example: C417 rule suggest that map here is unnecessary docs_with_tokens = list(map(lambda d: (_length_function(d.page_content), d), docs)) # noqa: C417
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: index-backup-restore
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - name: Install uv | ||
| run: | | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| echo "$HOME/.local/bin" >> $GITHUB_PATH |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual workflow file to confirm the current implementation
find . -type f -name "lint.yml" -path "*/.github/workflows/*"Repository: linagora/openrag
Length of output: 88
🌐 Web query:
astral-sh/setup-uv GitHub Action documentation
💡 Result:
I found the official documentation and README for the astral-sh/setup-uv GitHub Action:
-
Repository README (usage, inputs, outputs, advanced config, FAQ). Example install: uses: astral-sh/setup-uv@v7. Includes inputs like version, version-file, python-version, activate-environment, working-directory, checksum; outputs like uv-version, uv-path, venv, cache-hit. [1]
-
Astral docs guide for using uv in GitHub Actions (recommendation and example, showing pinning uv via the action). [2]
-
GitHub Marketplace entry for the action. [3]
If you want, I can paste the README sections (usage, inputs, outputs) or produce a ready-to-drop workflow snippet tailored to your OS and Python matrix.
🌐 Web query:
uv package manager latest version 2025 2026
💡 Result:
As of Jan 15, 2026:
- Latest uv release: 0.9.25 — released Jan 13, 2026. [1][2]
- Latest release in 2025: 0.9.21 — released Dec 30, 2025. [3]
Sources: GitHub Releases (uv 0.9.25) [1], PyPI (uv 0.9.25) [2], GitHub release list showing 2025 tags (0.9.21 on Dec 30, 2025) [3].
🏁 Script executed:
# Let's examine the actual lint.yml workflow file
cat -n ./.github/workflows/lint.ymlRepository: linagora/openrag
Length of output: 1568
Use the official astral-sh/setup-uv action for better reproducibility and maintenance.
The curl | sh approach installs the latest uv version, which lacks reproducibility. Instead, use the astral-sh/setup-uv action (v7), which provides version pinning, automatic caching, and is maintained by Astral.
♻️ Suggested replacement
- - name: Install uv
- run: |
- curl -LsSf https://astral.sh/uv/install.sh | sh
- echo "$HOME/.local/bin" >> $GITHUB_PATH
-
- - name: Cache uv
- uses: actions/cache@v4
- with:
- path: |
- ~/.cache/uv
- key: uv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('**/pyproject.toml', '**/uv.lock') }}
- restore-keys: |
- uv-${{ runner.os }}-${{ matrix.python-version }}-
+ - name: Install uv
+ uses: astral-sh/setup-uv@v7
+ with:
+ version: "0.9.25"This eliminates the manual cache step since setup-uv handles caching automatically.
🤖 Prompt for AI Agents
In @.github/workflows/lint.yml around lines 26 - 29, Replace the manual curl
installer block that adds uv (the "Install uv" step) with the official
astral-sh/setup-uv action (use astral-sh/setup-uv@v7) to pin the version and
enable automatic caching; remove the manual $HOME/.local/bin append and any
separate caching steps since setup-uv manages them, and pass a pinned version
via the action's "version" input to ensure reproducible installs.
ac5502b to
a0838a2
Compare
Ahmath-Gadji
added
the
chore
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
automatic-evaluation-pipeline/benchmark.py (1)
294-307: Fix metric misalignment after filtering judge errors.
valid_ndcg_scores = nDCG_scores[: len(valid_scores)]and thechunks_countslice can misalign metrics if errors are interspersed. Filter nDCG and chunks with the same mask used forvalid_scores.
Line 294-307.✅ Suggested fix (aligned filtering)
- valid_scores = [(comp, prec) for comp, prec in llm_judge_scores if comp != "error"] - valid_ndcg_scores = nDCG_scores[: len(valid_scores)] # Match the filtered scores - - eval_results = pd.DataFrame( - valid_scores, - columns=["completion_evaluation", "precision_evaluation"], - ) - eval_results["nDCG"] = valid_ndcg_scores - chunks_count = [ - len(input_reference["chunks"]) - for input_reference in list_response_answer_reference[: len(valid_scores)] - ] - eval_results["n_chunks"] = chunks_count + valid_rows = [ + (comp, prec, ndcg, len(ref["chunks"])) + for (comp, prec), ndcg, ref in zip( + llm_judge_scores, nDCG_scores, list_response_answer_reference + ) + if comp != "error" + ] + eval_results = pd.DataFrame( + valid_rows, + columns=["completion_evaluation", "precision_evaluation", "nDCG", "n_chunks"], + )openrag/components/reranker.py (2)
41-46: Input documents are mutated in place.Line 44 modifies the original
Documentobjects by settingmetadata["relevance_score"]. This side effect could surprise callers who don't expect their input to be modified. Consider creating shallow copies of the documents before modifying metadata.Proposed fix
output = [] for rerank_res in rerank_result.results: doc = documents[rerank_res.index] - doc.metadata["relevance_score"] = rerank_res.relevance_score - output.append(doc) + # Create a copy to avoid mutating the input + reranked_doc = Document( + page_content=doc.page_content, + metadata={**doc.metadata, "relevance_score": rerank_res.relevance_score}, + ) + output.append(reranked_doc) return output
37-40: External API call lacks a timeout.The
Clientinitialization on line 12 does not configure a timeout. Without it,rerank.asyncio()calls can block indefinitely if the rerank service becomes unresponsive, exhausting the 5-slot semaphore and stalling the entire system. Configure a timeout viahttpx_argswhen creating the Client:self.client = Client( base_url=config.reranker["base_url"], httpx_args={"timeout": httpx.Timeout(30.0)} # Adjust value as needed )This pattern is already used elsewhere in the codebase for external API calls (llm.py, indexer loaders).
openrag/components/indexer/loaders/txt_loader.py (1)
51-54: Incorrect docstring: states "plain text files (.txt)" for MarkdownLoader.The docstring appears to be copied from
TextLoader. It should describe Markdown files instead.📝 Proposed fix
class MarkdownLoader(BaseLoader): """ - Loader for plain text files (.txt). + Loader for Markdown files (.md). """openrag/components/indexer/chunker/utils.py (1)
194-210: PotentialTypeErrorwhenlength_functionisNone.The
length_functionparameter is typed asCallable[[str], int] | Nonewith defaultNone, but lines 209–210 call it without a null check:
header_ntoks = length_function(header_text)groups_ntoks = [length_function(g) for g in group_texts]This will raise
TypeError: 'NoneType' object is not callableif the function is called without providing a length function. While all current call sites in the codebase explicitly providelength_function, the function signature permitsNone, creating a type safety contract violation.Fix: Either remove the
| Noneand default from the signature, or add a null check with a sensible default:def chunk_table( table_element: MDElement, chunk_size: int = 512, length_function: Callable[[str], int] | None = None, ) -> list[MDElement]: + if length_function is None: + length_function = len txt = clean_markdown_table_spacing(table_element.content)openrag/api.py (1)
217-226: Duplicate router registration when both flags are enabled.When
WITH_OPENAI_API=TrueandWITH_CHAINLIT_UI=True, theopenai_routeris mounted twice to/v1. This causes duplicate route definitions and may lead to unexpected behavior.Proposed fix
if WITH_OPENAI_API: # Mount the openai router app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) if WITH_CHAINLIT_UI: # Mount the default front from chainlit.utils import mount_chainlit mount_chainlit(app, "./app_front.py", path="/chainlit") - app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) # cause chainlit uses openai api endpoints + if not WITH_OPENAI_API: + # Mount openai router for chainlit if not already mounted + app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI])openrag/components/indexer/chunker/chunker.py (1)
141-161:llm_configdefault ofNonewill causeTypeErrorat runtime.The parameter
llm_config: dict | None = Nonesuggests it's optional, but line 153 unconditionally unpacks it withChatOpenAI(**llm_config). Ifllm_configisNone, this raisesTypeError: argument after ** must be a mapping, not NoneType.Proposed fix — add validation or remove None default
Option 1: Add validation
def __init__( self, chunk_size: int = 200, chunk_overlap_rate: float = 0.2, llm_config: dict | None = None, contextual_retrieval: bool = False, **kwargs, ): + if llm_config is None: + raise ValueError("llm_config is required") + self.chunk_size = chunk_sizeOption 2: Remove None default (preferred if always required)
def __init__( self, chunk_size: int = 200, chunk_overlap_rate: float = 0.2, - llm_config: dict | None = None, + llm_config: dict, contextual_retrieval: bool = False, **kwargs, ):openrag/components/indexer/indexer.py (1)
61-80: Guarduserbefore dereference on line 79, or declare it as required.Line 79 calls
user.get("id")unconditionally, butuseris typed asdict | None = None. WhenuserisNone, this raisesAttributeError. Either add a guard before line 79 and handle the None case, or make theuserparameter required (remove the defaultNone). Note that line 96 also passesusertoinsert_documents.remote(), so ensure consistency across both call sites.
🤖 Fix all issues with AI agents
In `@automatic-evaluation-pipeline/benchmark.py`:
- Around line 265-267: The recall computation can raise ZeroDivisionError when
chunk_id_reference is empty; before calculating recall in the block that uses
chunk_id_reference and openrag_chunk_ids (where recall =
len(list(set(chunk_id_reference) & set(openrag_chunk_ids))) /
len(chunk_id_reference)), add a defensive guard that checks if
chunk_id_reference is empty and sets recall to 0.0 (or an appropriate sentinel)
instead of performing the division; keep the rest of the logic unchanged and
ensure references to chunk_id_reference, openrag_chunk_ids, and the recall
variable are used so the check is colocated with the original computation.
In `@LINTING.md`:
- Around line 140-143: Fix the minor grammar in the troubleshooting example
sentence by changing "suggest" to "suggests" in the line referencing the C417
rule (the example comment starting with "# Example: C417 rule suggest that map
here is unnecessary"); update that sentence so it reads "...C417 rule suggests
that map here is unnecessary".
In `@openrag/components/indexer/indexer.py`:
- Around line 329-331: get_all_states currently declares a return type of
dict[str, str] but iterates TaskInfo.state which is typed str | None, so change
the return annotation to dict[str, str | None] and keep the implementation the
same; update the get_all_states signature to reflect dict[str, str | None] and
ensure callers expect optional values from TaskInfo.state (refer to
get_all_states, TaskInfo.state, self.tasks and self.lock to locate the change).
- Around line 213-216: The asearch() method currently uses a mutable default for
the filter parameter (filter: dict | None = {}), which can cause state leakage
across calls; change the signature to accept filter: dict | None = None and
inside asearch (near where partition_list is obtained via _check_partition_list)
initialize a local filters variable with filter or {} (e.g., filters = filter or
{}) before using it so downstream mutations do not affect future calls; update
any references in asearch that expect the old parameter name accordingly.
In `@openrag/components/indexer/loaders/base.py`:
- Around line 94-96: The image-size check currently logs "Image too small..."
but falls through; after the logger.debug inside the conditional that checks
width/height against self.min_width_pixels/self.min_height_pixels, add an early
return (e.g., return None) to stop further description processing for small
images so the method actually skips them; update the containing method (the
function where this if appears) to consistently return the same sentinel (None
or empty string) when skipping.
In `@openrag/components/indexer/loaders/serializer.py`:
- Around line 42-43: The parameter metadata currently uses a mutable default {}
but is typed as dict | None; change the signature to use metadata: dict | None =
None and inside the function (e.g., at start of the function that declares path:
str | Path, metadata: ...) normalize it with something like metadata = metadata
or {} so subsequent calls to metadata.get(...) are safe and you avoid shared
mutable defaults; update the type hint and ensure all uses of metadata assume a
dict after normalization.
In `@openrag/scripts/filter-logs.py`:
- Around line 71-76: The code uses the name UTC (in the assume_tz assignment and
the else branch checking args.tz) which requires Python 3.11; change those uses
to the stdlib timezone.utc and ensure datetime.timezone is imported (replace
references to UTC with timezone.utc wherever assume_tz or the tz fallback is
set, and update the import to include from datetime import timezone or use
datetime.timezone). Target symbols: assume_tz, args.tz, UTC -> timezone.utc,
datetime.now().astimezone().tzinfo.
♻️ Duplicate comments (2)
openrag/components/indexer/loaders/base.py (1)
38-45: Type hints updated correctly;selfparameter now present.The abstract method signature is correct with
selfincluded and modernized type hints.openrag/routers/indexer.py (1)
282-282: Theuserparameter is now correctly passed toadd_file.remote().This addresses the previous review feedback about the missing
userparameter in theput_fileendpoint.
🧹 Nitpick comments (9)
openrag/scripts/backup.py (1)
268-270: Consider Pythonic empty-check idiom.The Yoda-style comparison
0 == len(partitions)works but is non-idiomatic in Python. Consider usingnot partitionsorlen(partitions) == 0for readability.Suggested change
- if 0 == len(partitions): + if not partitions: logger.error("No partitions meet given conditions.") return 1.github/workflows/api_tests/mock_vllm.py (1)
124-128: Minor type hint inconsistency.The function signature declares
text: str, but the implementation includes a fallback branch for non-string content (line 128). While this defensive coding is reasonable given thatmsg.contentcan be a list for vision models, the type hint could be more accurate.Optional: Align type hint with implementation
-def count_tokens(text: str) -> int: +def count_tokens(text: str | Any) -> int: """Approximate token count (roughly 4 chars per token).""" if isinstance(text, str): return max(1, len(text) // 4) return 10 # Default for non-string contentAlternatively, since callers already use
str(msg.content), the non-string branch may be dead code and could be removed.openrag/components/reranker.py (3)
10-17: Consider adding type hints to__init__parameters.Given this PR modernizes type annotations, the
loggerandconfigparameters could benefit from type hints for consistency.Suggested improvement
- def __init__(self, logger, config): + def __init__(self, logger: "structlog.BoundLogger", config: "Config"):Alternatively, if the concrete types aren't easily importable, use a protocol or
typing.Anywith a docstring.
26-26: Consider early return for empty document list.If
documentsis empty, the code still makes an external API call. An early return would avoid unnecessary network overhead.Suggested improvement
+ if not documents: + return [] top_k = min(top_k, len(documents))
55-55: Use bareraiseto preserve the full traceback.Using
raise eresets the traceback origin to this line. A bareraisepreserves the original exception context.Proposed fix
- raise e + raiseLINTING.md (1)
86-107: Handle staged filenames with spaces safely in the pre-commit hook.
Unquoted$FILESwill break on paths with spaces/newlines. Consider a null-delimited list and array expansion.♻️ Suggested doc update
-FILES=$(git diff --cached --name-only --diff-filter=ACM | grep '\.py$') +mapfile -d '' FILES < <(git diff --cached --name-only --diff-filter=ACM -z | grep -z '\.py$') -if [ -z "$FILES" ]; then +if [ ${`#FILES`[@]} -eq 0 ]; then exit 0 fi ... -uv run ruff check $FILES --fix +uv run ruff check "${FILES[@]}" --fix ... -uv run ruff format $FILES +uv run ruff format "${FILES[@]}" ... -git add $FILES +git add -- "${FILES[@]}" ... -uv run ruff check $FILES +uv run ruff check "${FILES[@]}"openrag/routers/indexer.py (1)
310-317: Type hints updated correctly; consider usingdictinstead ofAny.The modernization from
Optional[Any]toAny | Noneis correct. However, sincevalidate_metadatareturns a parsed JSON object (dict), usingdict | Nonewould provide better type safety.♻️ Optional: More precise type hint
async def patch_file( partition: str, file_id: str = Depends(validate_file_id), - metadata: Any | None = Depends(validate_metadata), + metadata: dict | None = Depends(validate_metadata),Apply similar change to
copy_file_between_partitions.Also applies to: 355-364
openrag/components/indexer/indexer.py (2)
52-52: Typetask_idas optional to match the default.
task_iddefaults toNonebut is typed asstr. Usestr | Noneto align with runtime behavior and the chunker signature.♻️ Proposed change
- async def chunk(self, doc: Document, file_path: str, task_id: str = None) -> list[Document]: + async def chunk(self, doc: Document, file_path: str, task_id: str | None = None) -> list[Document]:
58-66: PreferNonecheck overmetadata = metadata or {}.Using
or {}replaces a caller-supplied empty dict, so mutations won’t reflect back to the caller. Use an explicitNonecheck.♻️ Proposed change
- metadata = metadata or {} + if metadata is None: + metadata = {}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
.github/workflows/api_tests/mock_vllm.py.github/workflows/lint.ymlLINTING.mdautomatic-evaluation-pipeline/benchmark.pyautomatic-evaluation-pipeline/generate_questions.pyautomatic-evaluation-pipeline/upload_files.pyopenrag/api.pyopenrag/app_front.pyopenrag/chainlit_api.pyopenrag/components/indexer/chunker/chunker.pyopenrag/components/indexer/chunker/utils.pyopenrag/components/indexer/embeddings/__init__.pyopenrag/components/indexer/embeddings/openai.pyopenrag/components/indexer/indexer.pyopenrag/components/indexer/loaders/__init__.pyopenrag/components/indexer/loaders/base.pyopenrag/components/indexer/loaders/eml_loader.pyopenrag/components/indexer/loaders/image.pyopenrag/components/indexer/loaders/pdf_loaders/docling2.pyopenrag/components/indexer/loaders/pdf_loaders/marker.pyopenrag/components/indexer/loaders/pdf_loaders/openai.pyopenrag/components/indexer/loaders/serializer.pyopenrag/components/indexer/loaders/test_media_loader.pyopenrag/components/indexer/loaders/txt_loader.pyopenrag/components/indexer/utils/files.pyopenrag/components/indexer/utils/test_text_sanitizer.pyopenrag/components/indexer/vectordb/utils.pyopenrag/components/indexer/vectordb/vectordb.pyopenrag/components/llm.pyopenrag/components/map_reduce.pyopenrag/components/pipeline.pyopenrag/components/prompts/prompts.pyopenrag/components/reranker.pyopenrag/components/retriever.pyopenrag/components/utils.pyopenrag/models/indexer.pyopenrag/models/openai.pyopenrag/routers/actors.pyopenrag/routers/extract.pyopenrag/routers/indexer.pyopenrag/routers/openai.pyopenrag/routers/partition.pyopenrag/routers/queue.pyopenrag/routers/search.pyopenrag/routers/tools.pyopenrag/routers/users.pyopenrag/routers/utils.pyopenrag/scripts/backup.pyopenrag/scripts/embed.pyopenrag/scripts/filter-logs.pyopenrag/scripts/migrations/alembic/versions/4add4d260575_initial_migration.pyopenrag/scripts/migrations/alembic/versions/cd642e4502d8_create_users_memberships_tables.pyopenrag/scripts/restore.pyopenrag/utils/dependencies.pyopenrag/utils/test_external_resource_errors.pypyproject.tomltests/api_tests/conftest.pytests/api_tests/test_actors.pytests/api_tests/test_extract.pytests/api_tests/test_indexer.pytests/api_tests/test_openai_compat.pytests/api_tests/test_partition.pytests/api_tests/test_queue.pytests/api_tests/test_search.pytests/api_tests/test_users.pytests/test_vectordb.pyutility/data_indexer.py
💤 Files with no reviewable changes (6)
- openrag/utils/test_external_resource_errors.py
- tests/api_tests/test_actors.py
- tests/api_tests/test_users.py
- tests/api_tests/test_extract.py
- tests/api_tests/test_partition.py
- tests/api_tests/test_queue.py
✅ Files skipped from review due to trivial changes (4)
- openrag/routers/extract.py
- utility/data_indexer.py
- openrag/components/indexer/loaders/test_media_loader.py
- openrag/scripts/embed.py
🚧 Files skipped from review as they are similar to previous changes (27)
- openrag/routers/utils.py
- openrag/components/pipeline.py
- tests/test_vectordb.py
- openrag/routers/tools.py
- openrag/routers/partition.py
- automatic-evaluation-pipeline/upload_files.py
- pyproject.toml
- openrag/components/indexer/loaders/pdf_loaders/docling2.py
- openrag/components/indexer/loaders/image.py
- tests/api_tests/test_openai_compat.py
- openrag/app_front.py
- openrag/components/map_reduce.py
- openrag/components/llm.py
- openrag/models/indexer.py
- automatic-evaluation-pipeline/generate_questions.py
- openrag/components/retriever.py
- tests/api_tests/test_search.py
- openrag/components/utils.py
- openrag/chainlit_api.py
- openrag/scripts/restore.py
- openrag/utils/dependencies.py
- openrag/components/indexer/loaders/init.py
- tests/api_tests/test_indexer.py
- openrag/routers/queue.py
- openrag/routers/search.py
- .github/workflows/lint.yml
- openrag/components/indexer/loaders/pdf_loaders/openai.py
🧰 Additional context used
🧬 Code graph analysis (11)
openrag/components/indexer/utils/test_text_sanitizer.py (2)
openrag/components/indexer/chunker/test_chunking.py (1)
test_multiline_spacing(184-191)openrag/components/indexer/utils/text_sanitizer.py (1)
clean_markdown_table_spacing(98-127)
openrag/components/indexer/utils/files.py (1)
openrag/components/ray_utils.py (1)
call_ray_actor_with_timeout(11-57)
openrag/routers/users.py (2)
openrag/components/indexer/vectordb/utils.py (1)
delete_user(424-431)openrag/components/indexer/vectordb/vectordb.py (1)
delete_user(875-882)
openrag/routers/indexer.py (2)
openrag/components/indexer/indexer.py (4)
add_file(56-125)set_state(267-270)set_object_ref(299-302)copy_file(176-205)openrag/routers/utils.py (2)
validate_metadata(200-208)require_task_owner(156-170)
openrag/scripts/backup.py (4)
openrag/components/indexer/vectordb/utils.py (2)
list_partition_files(181-204)list_partitions(295-299)openrag/utils/logger.py (1)
get_logger(10-50)openrag/components/indexer/vectordb/vectordb.py (4)
list_partition_files(57-58)list_partition_files(718-735)list_partitions(45-46)list_partitions(737-742)openrag/scripts/embed.py (1)
open_output_file(246-272)
openrag/components/indexer/chunker/chunker.py (2)
openrag/components/indexer/chunker/utils.py (3)
MDElement(20-34)split_md_elements(56-110)get_chunk_page_number(113-148)openrag/components/indexer/embeddings/base.py (1)
BaseEmbedding(5-20)
openrag/components/indexer/loaders/txt_loader.py (2)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(19-159)openrag/utils/logger.py (1)
get_logger(10-50)
openrag/components/indexer/loaders/base.py (1)
openrag/utils/logger.py (1)
get_logger(10-50)
openrag/components/indexer/loaders/eml_loader.py (14)
openrag/components/indexer/loaders/__init__.py (1)
get_loader_classes(18-51)openrag/components/indexer/loaders/base.py (3)
BaseLoader(19-159)aload_document(39-45)get_image_description(68-159)openrag/components/indexer/loaders/image.py (1)
aload_document(13-20)openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
aload_document(108-133)openrag/components/indexer/loaders/pdf_loaders/marker.py (1)
aload_document(217-269)openrag/components/indexer/loaders/pdf_loaders/openai.py (1)
aload_document(42-69)openrag/components/indexer/loaders/txt_loader.py (2)
aload_document(26-48)aload_document(65-119)openrag/components/indexer/loaders/pptx_loader.py (1)
aload_document(151-167)openrag/components/indexer/loaders/media_loader.py (1)
aload_document(149-175)openrag/components/indexer/loaders/doc.py (1)
aload_document(17-31)openrag/components/indexer/loaders/pdf_loaders/pymupdf.py (2)
aload_document(14-29)aload_document(36-48)openrag/components/indexer/loaders/pdf_loaders/docling.py (1)
aload_document(65-90)openrag/components/indexer/loaders/CustomDocLoader.py (1)
aload_document(22-41)openrag/components/indexer/loaders/CustomHTMLLoader.py (1)
aload_document(13-22)
openrag/components/indexer/vectordb/utils.py (1)
openrag/components/indexer/vectordb/vectordb.py (3)
list_partition_files(57-58)list_partition_files(718-735)get_user_by_token(887-888)
openrag/components/indexer/indexer.py (3)
openrag/components/indexer/chunker/chunker.py (1)
split_document(288-314)openrag/routers/indexer.py (1)
add_file(115-172)tests/api_tests/test_indexer.py (1)
get_task_id(43-49)
🪛 LanguageTool
LINTING.md
[uncategorized] ~68-~68: The official name of this software platform is spelled with a capital “H”.
Context: ...Integration A GitHub Actions workflow (.github/workflows/lint.yml) automatically runs...
(GITHUB)
🔇 Additional comments (58)
openrag/components/indexer/embeddings/__init__.py (1)
21-21: LGTM — export list reorder is safe.
No functional impact; the module’s public surface remains the same.openrag/components/indexer/embeddings/openai.py (1)
44-69: LGTM!The conversion from
{str(e)}to{e!s}is the idiomatic Python f-string syntax and aligns with Ruff's formatting preferences. Both changes are consistent and maintain identical behavior.openrag/components/indexer/utils/files.py (1)
73-78: LGTM — safer default formetadata.
Switching the default toNoneand normalizing to a dict removes the mutable-default pitfall while keeping downstream usage consistent.openrag/scripts/backup.py (6)
3-10: LGTM!Imports are properly organized following PEP 8 conventions (standard library → third-party → local), and the
typingimports (IO,Any) are correctly included for the modernized type hints.
13-69: LGTM!Type hints are correctly modernized to Python 3.10+ syntax (
dict[str, dict[str, Any]]). The logic for iterating partitions, fetching files, and writing JSON output is sound with appropriate error handling.
72-137: LGTM!Type hints correctly modernized. The iterator pattern for batch processing Milvus data is well-implemented with proper error handling. The verbose counter (
cnt) is correctly scoped within the verbose condition blocks.
140-168: LGTM!Implementation is consistent with
openrag/scripts/embed.py(same pattern for stdout handling, file existence check, and xz compression support). The function signature with modern type hints is clean.
291-309: LGTM!Proper use of context manager for file handling ensures resources are cleaned up correctly. The explicit
flush()before exit and comprehensive exception handling withlogger.exceptionare good practices.
314-315: LGTM!Standard entry point pattern with proper exit code propagation.
openrag/scripts/filter-logs.py (3)
13-13: LGTM!The type hint modernization from
Optional[datetime]todatetime | Nonecorrectly follows Python 3.10+ union syntax and aligns with the PR objectives.
87-88: Minor: Removal of explicit read mode is fine.Removing
"r"fromopen()has no functional impact as read mode is the default. This is acceptable.
5-5: No compatibility issue exists. The project requires Python 3.12+ (requires-python = ">=3.12"), which is higher than Python 3.11 whendatetime.UTCwas introduced. Thedatetime.UTCimport is fully compatible with the project's minimum Python version.Likely an incorrect or invalid review comment.
openrag/models/openai.py (3)
1-1: LGTM on reduced typing imports.The import statement correctly retains only
AnyandLiteralfrom the typing module, as the other types (Optional,List,Dict,Union) are now replaced with Python 3.10+ built-in syntax.
14-30: LGTM on type modernization.The class fields are correctly updated to Python 3.10+ union syntax. The mutable default dict for
metadatais properly handled by Pydantic'sField()which creates a copy for each instance.
33-50: LGTM on type modernization.The legacy completion request fields are correctly updated. The
dict | Noneforlogit_biasis acceptable, though you could optionally add type parameters (dict[str, float] | None) to match the OpenAI API specification more precisely..github/workflows/api_tests/mock_vllm.py (6)
23-39: LGTM on embedding models.The type hints correctly reflect the OpenAI embeddings API patterns: accepting single or batch string inputs, returning float vectors, and wrapping in a list response.
50-79: LGTM on chat completion models.The modernized type hints accurately reflect the OpenAI chat completions API. The
stop: str | list[str] | Nonecorrectly allows both single and multiple stop sequences.
85-108: LGTM on text completion models.Type hints correctly reflect the legacy OpenAI completions API with support for both single and batch prompts.
114-121: LGTM on helper function.Using MD5 for deterministic fake embedding generation is appropriate for testing purposes. The function correctly produces consistent embeddings based on text hash.
131-156: LGTM on mock response generation.The function correctly handles both string and vision model (list) content formats, generating contextual mock responses for different query types.
179-247: LGTM on API endpoints.All endpoints correctly use the updated Pydantic models with modernized type hints. The implementation properly handles both single and batch inputs for embeddings and completions.
automatic-evaluation-pipeline/benchmark.py (6)
4-12: Import additions look fine.
105-110: Formatting-only change; no issues spotted.
118-123: Formatting-only change; no issues spotted.
222-230: No issues in dataset load/partition tweak.
256-258: Refactor-only change; looks OK.
322-330: Output formatting tweak looks fine.tests/api_tests/conftest.py (2)
4-4: Spacing after the module docstring is fine.
Improves readability without changing behavior.
90-92: Assertion formatting looks good.
Multiline message is clearer, no behavior change.openrag/components/prompts/prompts.py (1)
25-26: LGTM — default read mode is acceptable here.
No behavior change from explicit"r".LINTING.md (1)
1-82: Doc structure and commands are clear.
Nice, concise onboarding steps for Ruff.openrag/routers/openai.py (1)
189-192: Consistent exception string formatting — looks good.
No behavioral change; just standardized formatting.Also applies to: 240-243, 318-320, 333-335
openrag/routers/actors.py (1)
33-36: Formatting cleanup is fine.
Multiline decorators and {e!s} details are consistent with the rest of the codebase.Also applies to: 74-77, 125-126, 145-146
openrag/components/indexer/utils/test_text_sanitizer.py (2)
127-127: LGTM - Assertion formatting consolidated.The multi-line assertions are correctly collapsed to single lines for linting compliance without changing the expected values.
Also applies to: 141-141, 148-148
187-187: LGTM - noqa comment preserves intentional test artifact.The
# noqa: W293comment appropriately suppresses the trailing whitespace warning since this test string intentionally simulates PDF extraction artifacts with whitespace issues.openrag/components/indexer/loaders/serializer.py (1)
88-88: LGTM - Bare raise preserved.The bare
raisecorrectly re-raises the caught exception after logging.openrag/components/indexer/loaders/pdf_loaders/marker.py (1)
217-226: LGTM - Type hints correctly modernized.The
metadata: dict | None = Nonepattern with explicit None-handling at lines 225-226 is the correct approach. This is the pattern that should be followed inserializer.pyas well.openrag/routers/users.py (2)
13-14: LGTM - Route decorator formatting improved.Placing the path argument on a separate line improves readability when combined with multi-line description strings.
Also applies to: 37-38, 60-61, 102-103, 133-134, 169-170
83-89: LGTM - Type hints modernized for Form parameters.The
str | None = Form(None)syntax is correct for optional form fields in FastAPI.openrag/scripts/migrations/alembic/versions/4add4d260575_initial_migration.py (1)
9-9: LGTM - Import and type hints modernized.Using
collections.abc.Sequenceinstead oftyping.Sequenceis the preferred approach in Python 3.9+, and the union syntax is correctly updated for Python 3.10+.Also applies to: 33-35
openrag/scripts/migrations/alembic/versions/cd642e4502d8_create_users_memberships_tables.py (1)
9-9: LGTM! Type hint modernization looks correct.The migration from
typing.Sequenceandtyping.Uniontocollections.abc.Sequenceand Python 3.10+ union syntax (str | Sequence[str] | None) is appropriate and aligns with modern Python practices.Also applies to: 33-35
openrag/components/indexer/loaders/base.py (1)
68-71: Type hint modernization looks good.The parameter type
Image.Image | strcorrectly uses Python 3.10+ union syntax.openrag/components/indexer/loaders/txt_loader.py (1)
26-31: LGTM! Type hints correctly modernized and consistent with base class.Both
TextLoaderandMarkdownLoaderimplementations use the updated Python 3.10+ union syntax matchingBaseLoader.aload_document.Also applies to: 65-70
openrag/components/indexer/chunker/utils.py (1)
2-3: LGTM! Type hint modernization is correct.Using
Callablefromcollections.abcandLiteralfromtypingis appropriate, and the union syntax forpage_number: int | Noneis valid.Also applies to: 23-28
openrag/components/indexer/vectordb/utils.py (2)
181-211: LGTM! Type hints modernized consistently.The type annotation updates to Python 3.10+ union syntax (
int | None,dict | None) are correctly applied and align with the corresponding interface invectordb.py.
334-401: Type hint modernization applied correctly to user-related methods.The return type annotations (
dict | None) and parameter types (str | None) are consistent with the function behavior that returnsNonewhen entities are not found.openrag/components/indexer/loaders/eml_loader.py (4)
30-30: Type annotation corrected.The signature now correctly uses
dict | None = None, fixing the previous inconsistency where the type wasdictbut the default wasNone.
193-194: Keyword argument corrected.The call now correctly uses
image_data=imagematching the base classget_image_descriptionparameter name.
204-206: Short-circuit evaluation makes this safe.The condition
file_ext == ".pdf" and not fallback_successis safe because Python's short-circuit evaluation ensuresfallback_successis only evaluated whenfile_ext == ".pdf", which is precisely whenfallback_successis defined (line 153).
237-238: Keyword argument corrected.The call now correctly uses
image_data=imagematching the base class method signature.openrag/api.py (1)
76-77: Type annotations corrected.The type hints are now correctly
boolinstead ofbool | None, matching the actual return type of the comparison expression.openrag/components/indexer/chunker/chunker.py (3)
215-215: Type hint modernization applied correctly.
288-288: Type hint modernization applied correctly.
345-348: Type hint modernization applied correctly.The
embedder: BaseEmbedding | None = Noneparameter correctly indicates it's optional.openrag/components/indexer/vectordb/vectordb.py (4)
56-97: Abstract method signatures modernized consistently.The type hint updates (
int | None,dict | None,list[Document]) inBaseVectorDBalign correctly with the concrete implementations inMilvusDB.
332-333: Return type annotation modernized.
list[str]correctly uses Python 3.10+ built-in generic syntax.
434-441: Type hints modernized in async_search.The
filter: dict | None = Noneparameter type is consistent with the abstract method signature.
718-721: Implementation aligns with abstract method signature.The
limit: int | None = Noneparameter correctly matches the updatedBaseVectorDB.list_partition_filessignature.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| async def get_all_states(self) -> dict[str, str]: | ||
| async with self.lock: | ||
| return {tid: info.state for tid, info in self.tasks.items()} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the TaskInfo class definition and examine its state attribute
rg -n "class TaskInfo" openrag/components/indexer/Repository: linagora/openrag
Length of output: 117
🏁 Script executed:
# Also search for any type hints related to TaskInfo.state
rg -n "state.*:" openrag/components/indexer/indexer.py | head -20Repository: linagora/openrag
Length of output: 375
🏁 Script executed:
# Get a broader view of the indexer.py file structure
wc -l openrag/components/indexer/indexer.pyRepository: linagora/openrag
Length of output: 101
🏁 Script executed:
# View the TaskInfo class definition to confirm state type annotation
sed -n '246,260p' openrag/components/indexer/indexer.pyRepository: linagora/openrag
Length of output: 555
Return type should allow None states.
TaskInfo.state is typed as str | None, so get_all_states can return None values in the dictionary. The return type must be dict[str, str | None].
♻️ Proposed change (type fix)
- async def get_all_states(self) -> dict[str, str]:
+ async def get_all_states(self) -> dict[str, str | None]:🤖 Prompt for AI Agents
In `@openrag/components/indexer/indexer.py` around lines 329 - 331, get_all_states
currently declares a return type of dict[str, str] but iterates TaskInfo.state
which is typed str | None, so change the return annotation to dict[str, str |
None] and keep the implementation the same; update the get_all_states signature
to reflect dict[str, str | None] and ensure callers expect optional values from
TaskInfo.state (refer to get_all_states, TaskInfo.state, self.tasks and
self.lock to locate the change).
- Apply ruff code linting (PEP 8, Pyflakes, isort, etc.) - Auto-format code to 120 character line length - Sort imports with isort rules - Modernize Python code with pyupgrade rules
formatting checks on all pull requests and pushes to main/dev branches.
a0838a2 to
04beee4
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
automatic-evaluation-pipeline/generate_questions.py (1)
57-65: Avoid creatingasyncio.Semaphorein default args (event-loop binding risk).Creating the semaphore at import time can bind it to the wrong event loop and trigger
RuntimeError: <Semaphore> is bound to a different event loopunderasyncio.run. Prefer constructing it inside a running loop (and ideally pass it in).✅ Safer pattern (create in running loop)
-async def summarize( - chunk: str, semaphore: asyncio.Semaphore = asyncio.Semaphore(10) -) -> str: +async def summarize( + chunk: str, semaphore: asyncio.Semaphore | None = None +) -> str: + if semaphore is None: + semaphore = asyncio.Semaphore(10) async with semaphore: message = [ { "role": "user", "content": f"Voici le document:\n{chunk}. Donnez-moi un résumé qui précise quel type d'informations et de contenu contient le passage, mais sans entrer dans des détails trop précis.", } ]Please mirror the same change in
question_answerto avoid the same pitfall there.openrag/api.py (1)
217-226: Avoid double-registering the OpenAI router when Chainlit is enabled.When both
WITH_OPENAI_APIandWITH_CHAINLIT_UIflags are true, the router is registered twice with identical prefix and tags, creating duplicate routes in the FastAPI application and OpenAPI schema. Combine the conditions to ensure registration happens only once.Proposed fix
-if WITH_OPENAI_API: - # Mount the openai router - app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) +if WITH_OPENAI_API or WITH_CHAINLIT_UI: + # Mount the openai router (needed for OpenAI API and Chainlit) + app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) @@ if WITH_CHAINLIT_UI: # Mount the default front from chainlit.utils import mount_chainlit mount_chainlit(app, "./app_front.py", path="/chainlit") - app.include_router(openai_router, prefix="/v1", tags=[Tags.OPENAI]) # cause chainlit uses openai api endpoints + # openai_router already included above for Chainlit useopenrag/components/indexer/indexer.py (1)
58-66: Fix mutable default formetadatainserialize_file.The method uses a shared
{}default which violates Python best practices and is inconsistent with the pattern already used inadd_file(line 71) and the importedserialize_fileutility function. Change tometadata: dict | None = Nonewithmetadata = metadata or {}inside the method.Suggested fix
async def serialize_file( self, path: str, - metadata: dict = {}, + metadata: dict | None = None, task_id: str = None, ): + metadata = metadata or {} # Serialize doc = await serialize_file(task_id, path, metadata=metadata)
🤖 Fix all issues with AI agents
In `@automatic-evaluation-pipeline/benchmark.py`:
- Around line 256-263: Guard against empty input_reference["chunks"] before
indexing chunk_id_reference[0]: if chunk_id_reference is empty, append fallback
values (e.g., 0) to hit_rates and MRRs instead of calling
compute_hits/compute_inverted_ranks; otherwise call
compute_hits(chunk_id_reference[0], openrag_chunk_ids) and
compute_inverted_ranks(chunk_id_reference[0], openrag_chunk_ids) as before.
Ensure this check updates the same lists hit_rates and MRRs and uses the
variable names chunk_id_reference and openrag_chunk_ids so the logic is
localized to this block.
In `@automatic-evaluation-pipeline/generate_questions.py`:
- Around line 1-15: The project is missing runtime dependencies for the imports
"numpy" and "tqdm" used in generate_questions.py; open the project's
pyproject.toml and add entries for numpy (e.g., numpy>=1.24.0) and tqdm (e.g.,
tqdm>=4.66.0) to the top-level [project.dependencies] (or equivalent
dependencies list) alongside the existing hdbscan and httpx entries so the
imports in generate_questions.py (numpy, tqdm.asyncio.tqdm) resolve at runtime.
In `@openrag/components/indexer/utils/files.py`:
- Around line 73-77: The serialize_file function in indexer.py currently uses a
mutable default (metadata: dict = {}); change the parameter to accept None
(e.g., metadata: dict | None = None), then inside serialize_file normalize with
metadata = metadata or {} (or explicit if metadata is None: metadata = {});
update any type hints/usages accordingly to avoid shared mutable state and
mirror the fix applied in files.py.
In `@openrag/scripts/backup.py`:
- Around line 51-69: PartitionFileManager.list_partition_files() may return an
empty dict causing files["files"] KeyError; change the handling after calling
pfm.list_partition_files(part_name) to default files_list = files.get("files",
[]) (or similar) and use files_list for sorting, popping "partition", writing
JSON lines to out_fh, writing the separator, and logging the count (use
len(files_list)); keep the rest of the try/except and variable names
(pfm.list_partition_files, files, out_fh, verbose, logger) intact.
♻️ Duplicate comments (4)
LINTING.md (2)
140-140: Fix typo: "unecessary" → "unnecessary".This typo was flagged in a previous review but hasn't been corrected yet.
143-143: Fix grammar: "suggest" → "suggests".This grammar issue was flagged in a previous review but hasn't been corrected yet.
automatic-evaluation-pipeline/benchmark.py (1)
265-267: Guard against empty reference list to avoid ZeroDivisionError.This is the same issue previously flagged:
len(chunk_id_reference)can be zero. Please add a defensive guard here.openrag/components/indexer/indexer.py (1)
346-348: Return type should allowNonestates.
TaskInfo.stateisstr | None, soget_all_statesshould reflect that.♻️ Suggested fix
- async def get_all_states(self) -> dict[str, str]: + async def get_all_states(self) -> dict[str, str | None]:
🧹 Nitpick comments (6)
openrag/components/prompts/prompts.py (1)
25-27: LGTM — Consider adding explicit encoding for cross-platform consistency.The removal of the explicit
"r"mode is a valid stylistic simplification sinceopen()defaults to read mode. However, consider specifyingencoding="utf-8"to ensure consistent behavior across platforms, as prompt files may contain non-ASCII characters.♻️ Optional improvement
- with open(file_path) as f: + with open(file_path, encoding="utf-8") as f:LINTING.md (1)
57-57: Clarify or remove the RUF rules entry.The documentation lists "RUF: Ruff-specific rules" under "Key Rules Enabled" but notes they are "
not applied currently". This is contradictory and may confuse developers. Either remove RUF from the list if it's not enabled, or explain why it's documented but not applied.openrag/components/reranker.py (1)
11-17: Make the concurrency limit configurable (avoid magic number).Different deployments will want different parallelism. Consider reading a
max_concurrencyfrom config with a sane default and guard against non-positive values.♻️ Proposed refactor
- self.semaphore = asyncio.Semaphore( - 5 - ) # Only allow 5 reranking operation at a time + max_concurrency = max(1, int(config.reranker.get("max_concurrency", 5))) + self.semaphore = asyncio.Semaphore( + max_concurrency + ) # Only allow N reranking operations at a timeopenrag/components/map_reduce.py (1)
52-52: Consider updating the type annotation.The type hint
ChatOpenAIis technically inaccurate since.with_structured_output()returns aRunnable(specifically aRunnableSerializablethat outputsSummarizedChunk), not aChatOpenAIinstance. This could be misleading for IDE type checking and future maintainers.🔧 Suggested fix
- self.slm: ChatOpenAI = ChatOpenAI(**config.llm).with_structured_output(SummarizedChunk) + self.slm = ChatOpenAI(**config.llm).with_structured_output(SummarizedChunk)Or use a more accurate type if needed:
from langchain_core.runnables import Runnable self.slm: Runnable[list, SummarizedChunk] = ChatOpenAI(**config.llm).with_structured_output(SummarizedChunk)openrag/scripts/restore.py (1)
193-206: Consider adding explicitreturn 0for clarity.The docstring states the function returns
int, but success paths returnNoneimplicitly. Whilesys.exit(None)works correctly (exits with code 0), adding an explicitreturn 0at the end ofmain()would align with the documented contract.Proposed addition at end of main()
finally: client.close() return 0automatic-evaluation-pipeline/generate_questions.py (1)
146-151: Consider making the partition configurable instead of hard-coded.This avoids editing code for different runs and makes CI/local usage more flexible.
♻️ Example using an env var with fallback
- partition = "pdftest" # To replace with your wanted partition's name + partition = os.environ.get("PARTITION_NAME", "pdftest") # Override via env
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
.github/workflows/api_tests/mock_vllm.py.github/workflows/lint.ymlLINTING.mdautomatic-evaluation-pipeline/benchmark.pyautomatic-evaluation-pipeline/generate_questions.pyautomatic-evaluation-pipeline/upload_files.pyopenrag/api.pyopenrag/app_front.pyopenrag/chainlit_api.pyopenrag/components/indexer/chunker/chunker.pyopenrag/components/indexer/chunker/utils.pyopenrag/components/indexer/embeddings/__init__.pyopenrag/components/indexer/embeddings/openai.pyopenrag/components/indexer/indexer.pyopenrag/components/indexer/loaders/__init__.pyopenrag/components/indexer/loaders/base.pyopenrag/components/indexer/loaders/eml_loader.pyopenrag/components/indexer/loaders/image.pyopenrag/components/indexer/loaders/pdf_loaders/docling2.pyopenrag/components/indexer/loaders/pdf_loaders/marker.pyopenrag/components/indexer/loaders/pdf_loaders/openai.pyopenrag/components/indexer/loaders/serializer.pyopenrag/components/indexer/loaders/test_media_loader.pyopenrag/components/indexer/loaders/txt_loader.pyopenrag/components/indexer/utils/files.pyopenrag/components/indexer/utils/test_text_sanitizer.pyopenrag/components/indexer/vectordb/utils.pyopenrag/components/indexer/vectordb/vectordb.pyopenrag/components/llm.pyopenrag/components/map_reduce.pyopenrag/components/pipeline.pyopenrag/components/prompts/prompts.pyopenrag/components/reranker.pyopenrag/components/retriever.pyopenrag/components/utils.pyopenrag/models/indexer.pyopenrag/models/openai.pyopenrag/routers/actors.pyopenrag/routers/extract.pyopenrag/routers/indexer.pyopenrag/routers/openai.pyopenrag/routers/partition.pyopenrag/routers/queue.pyopenrag/routers/search.pyopenrag/routers/tools.pyopenrag/routers/users.pyopenrag/routers/utils.pyopenrag/scripts/backup.pyopenrag/scripts/embed.pyopenrag/scripts/filter-logs.pyopenrag/scripts/migrations/alembic/versions/4add4d260575_initial_migration.pyopenrag/scripts/migrations/alembic/versions/cd642e4502d8_create_users_memberships_tables.pyopenrag/scripts/restore.pyopenrag/utils/dependencies.pyopenrag/utils/logger.pyopenrag/utils/test_external_resource_errors.pyopenrag/utils/test_logger.pypyproject.tomltests/api_tests/conftest.pytests/api_tests/test_actors.pytests/api_tests/test_extract.pytests/api_tests/test_indexer.pytests/api_tests/test_openai_compat.pytests/api_tests/test_partition.pytests/api_tests/test_queue.pytests/api_tests/test_search.pytests/api_tests/test_users.py
💤 Files with no reviewable changes (8)
- tests/api_tests/test_users.py
- openrag/utils/test_external_resource_errors.py
- openrag/utils/test_logger.py
- tests/api_tests/test_actors.py
- tests/api_tests/test_extract.py
- tests/api_tests/test_queue.py
- tests/api_tests/test_partition.py
- openrag/utils/logger.py
✅ Files skipped from review due to trivial changes (1)
- openrag/components/indexer/loaders/test_media_loader.py
🚧 Files skipped from review as they are similar to previous changes (15)
- openrag/app_front.py
- openrag/routers/tools.py
- openrag/scripts/filter-logs.py
- openrag/routers/utils.py
- .github/workflows/lint.yml
- openrag/utils/dependencies.py
- openrag/routers/queue.py
- openrag/components/pipeline.py
- openrag/components/utils.py
- openrag/scripts/embed.py
- pyproject.toml
- openrag/components/indexer/loaders/serializer.py
- tests/api_tests/test_openai_compat.py
- openrag/routers/extract.py
- openrag/components/indexer/chunker/utils.py
🧰 Additional context used
🧬 Code graph analysis (19)
openrag/components/indexer/utils/files.py (2)
openrag/components/indexer/indexer.py (1)
serialize_file(58-66)openrag/components/ray_utils.py (1)
call_ray_actor_with_timeout(11-57)
openrag/components/indexer/utils/test_text_sanitizer.py (2)
openrag/components/indexer/chunker/test_chunking.py (1)
test_multiline_spacing(184-191)openrag/components/indexer/utils/text_sanitizer.py (1)
clean_markdown_table_spacing(98-127)
openrag/components/indexer/loaders/eml_loader.py (3)
openrag/components/indexer/loaders/__init__.py (1)
get_loader_classes(18-51)openrag/components/indexer/loaders/base.py (3)
BaseLoader(19-151)aload_document(37-43)get_image_description(66-151)openrag/components/indexer/loaders/image.py (1)
aload_document(13-20)
tests/api_tests/test_indexer.py (2)
tests/api_tests/conftest.py (3)
api_client(16-19)created_partition(87-98)sample_text_file(39-53)tests/api_tests/test_tools.py (1)
pdf_file_path(13-17)
openrag/api.py (4)
openrag/config/config.py (1)
load_config(12-29)openrag/utils/dependencies.py (1)
get_vectordb(47-49)openrag/utils/exceptions/base.py (1)
OpenRAGError(4-24)openrag/utils/logger.py (1)
get_logger(13-57)
openrag/components/indexer/loaders/image.py (1)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(19-151)
openrag/components/indexer/loaders/__init__.py (1)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(19-151)
openrag/routers/users.py (2)
openrag/components/indexer/vectordb/utils.py (1)
delete_user(424-431)openrag/components/indexer/vectordb/vectordb.py (1)
delete_user(875-882)
openrag/components/indexer/loaders/txt_loader.py (1)
openrag/components/indexer/loaders/base.py (1)
BaseLoader(19-151)
openrag/components/indexer/chunker/chunker.py (2)
openrag/components/indexer/chunker/utils.py (3)
MDElement(20-34)split_md_elements(56-110)get_chunk_page_number(113-148)openrag/components/indexer/embeddings/base.py (1)
BaseEmbedding(5-20)
openrag/scripts/backup.py (3)
openrag/components/indexer/vectordb/utils.py (3)
PartitionFileManager(140-566)list_partition_files(181-204)list_partitions(295-299)openrag/components/indexer/vectordb/vectordb.py (4)
list_partition_files(57-58)list_partition_files(718-735)list_partitions(45-46)list_partitions(737-742)openrag/scripts/embed.py (1)
open_output_file(246-272)
openrag/components/indexer/vectordb/utils.py (1)
openrag/components/indexer/vectordb/vectordb.py (3)
list_partition_files(57-58)list_partition_files(718-735)get_user_by_token(887-888)
openrag/components/indexer/loaders/base.py (1)
openrag/utils/logger.py (1)
get_logger(13-57)
openrag/routers/indexer.py (2)
openrag/components/indexer/indexer.py (3)
add_file(68-141)set_state(284-287)copy_file(192-221)openrag/routers/utils.py (2)
validate_metadata(200-208)require_task_owner(156-170)
openrag/components/indexer/indexer.py (3)
openrag/components/indexer/chunker/chunker.py (1)
split_document(288-314)openrag/components/indexer/utils/files.py (1)
serialize_file(73-86)tests/api_tests/test_indexer.py (1)
get_task_id(43-49)
openrag/components/indexer/loaders/pdf_loaders/openai.py (2)
openrag/components/indexer/loaders/base.py (1)
aload_document(37-43)openrag/components/indexer/loaders/image.py (1)
aload_document(13-20)
openrag/components/indexer/vectordb/vectordb.py (4)
openrag/components/indexer/vectordb/utils.py (3)
list_partition_files(181-204)file_exists_in_partition(322-330)create_user(334-361)openrag/utils/exceptions/base.py (2)
VDBError(37-43)EmbeddingError(28-34)openrag/utils/exceptions/vectordb.py (4)
VDBConnectionError(4-13)UnexpectedVDBError(117-126)VDBSearchError(57-66)VDBDeleteError(43-54)openrag/routers/users.py (1)
create_user(83-99)
openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
openrag/config/config.py (1)
load_config(12-29)
openrag/scripts/restore.py (3)
openrag/components/indexer/indexer.py (1)
chunk(53-55)openrag/scripts/backup.py (1)
load_openrag_config(187-207)openrag/components/indexer/vectordb/vectordb.py (2)
list_partitions(45-46)list_partitions(737-742)
🪛 LanguageTool
LINTING.md
[uncategorized] ~68-~68: The official name of this software platform is spelled with a capital “H”.
Context: ...Integration A GitHub Actions workflow (.github/workflows/lint.yml) automatically runs...
(GITHUB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: api-tests
- GitHub Check: tests (3.12)
🔇 Additional comments (111)
openrag/components/indexer/utils/test_text_sanitizer.py (1)
122-128: Formatting-only test tidy-up looks good.These are readability/linting adjustments without behavior changes.
Also applies to: 136-142, 143-149, 171-188
tests/api_tests/conftest.py (2)
1-5: LGTM!Adding a blank line after the module docstring aligns with standard Python formatting conventions.
90-92: LGTM!The multi-line assertion using parentheses for implicit line continuation is a clean way to comply with line length limits while maintaining readability.
openrag/components/indexer/embeddings/__init__.py (1)
21-21: LGTM!The
__all__reordering to alphabetical order is a reasonable style improvement with no functional impact.openrag/components/indexer/embeddings/openai.py (2)
44-51: LGTM!Using
{e!s}instead of{str(e)}is the more idiomatic f-string syntax and produces identical output. Good consistency improvement.
62-69: LGTM!Consistent use of
{e!s}conversion specifier for exception formatting.automatic-evaluation-pipeline/upload_files.py (2)
1-6: LGTM! Import reorganization follows PEP 8 conventions.The imports are now correctly grouped: standard library (
os,pathlib) separated from third-party packages (httpx,dotenv,loguru) with a blank line, which aligns with Ruff/isort formatting rules.
17-17: Minor formatting change approved.Extra blank line is acceptable for readability and likely auto-formatted by Ruff.
openrag/components/map_reduce.py (4)
32-37: LGTM!Formatting change to consolidate the
relevancyfield declaration onto a single line improves readability while maintaining the same behavior.
64-84: LGTM!Formatting refinement to the
contentfield construction. The error handling and fallback return are appropriate.
86-114: LGTM!Formatting improvements throughout the method. The termination condition logic (Line 101) correctly determines early exit when the last
expansion_batch_sizechunks are all irrelevant. Debug logging behavior is preserved.
116-157: LGTM!Formatting refinements throughout the
mapmethod. The batch processing logic, termination conditions, and control flow remain functionally equivalent. The modernizedlist[Document]type hints (Line 127) align with the PR's Python 3.10+ migration objectives.openrag/scripts/restore.py (7)
15-25: LGTM! Type annotations modernized correctly.The function signature now uses Python 3.10+ built-in generics (
list[str] | None,dict[str, set[str]]) which aligns with the PR's type hint modernization objective.
59-75: LGTM! Formatting improvements.The single-line conditionals and condensed logging statements improve readability while preserving the original logic.
122-167: LGTM! Type annotation and formatting updates.The
added_documentstype annotation is consistent withread_rdb_section, and the condensed skip-logic preserves correct filtering behavior.
186-188: LGTM! Simplified file open call.Removing the explicit
"rt"mode is correct—open()defaults to read mode, and specifyingencodingimplies text mode.
208-228: LGTM! Return type annotation added.The
tuple[dict[str, Any], dict[str, Any]]return type accurately reflects the function's behavior and is consistent with the similar function inbackup.py.
250-251: Good fix!type=intadded for--user-id.This correctly ensures
args.user_idis an integer when passed toadd_file_to_partition, addressing the previously identified issue.
259-286: LGTM! Formatting improvements in main().The condensed dict comprehension (line 283) and single-line formatting for MilvusDB.options and error messages improve readability without changing functionality.
.github/workflows/api_tests/mock_vllm.py (6)
9-9: LGTM!Clean import trimming—only
Anyis retained since it's still needed forChatMessage.content. TheUnion,List, andOptionalimports are no longer required with Python 3.10+ built-in generics and union syntax.
50-79: LGTM!The
ChatCompletionRequestandChatCompletionResponsemodels are correctly updated with Python 3.10+ type syntax. The optional field patterntype | None = defaultis appropriate for fields that have defaults but can also explicitly acceptNone.
85-108: LGTM!Consistent modernization of
TextCompletionRequestandTextCompletionResponsemodels, mirroring the pattern used in the chat completion models.
114-131: LGTM!Function signatures correctly updated to use Python 3.10+ type syntax. The implementations remain compatible with these type annotations.
183-196: LGTM!The formatting changes inline the list comprehension and generator expression, improving readability while maintaining identical functionality. These align with Ruff's formatting conventions introduced in this PR.
23-39: Type hints are correct for Python 3.12+.The updates to
EmbeddingRequest,EmbeddingData, andEmbeddingResponsecorrectly use Python 3.10+ union and generic syntax. Since the project targets Python 3.12+ (requires-python = ">=3.12"in pyproject.toml), these type hints are natively supported at runtime without needingfrom __future__ import annotations. Pydantic is available as a transitive dependency and supports this syntax.openrag/chainlit_api.py (1)
1-6: Formatting-only import/order change looks good.No functional impact detected.
openrag/components/indexer/loaders/image.py (1)
1-6: Import reordering is fine.No behavior changes introduced.
openrag/routers/partition.py (1)
26-27: Decorator formatting-only changes look good.No functional impact observed.
Also applies to: 53-54, 80-81, 131-132, 179-180, 223-224, 255-256, 293-294, 334-335, 371-372
tests/api_tests/test_search.py (1)
2-2: Formatting tweaks look good.No behavioral change to tests.
Also applies to: 20-21, 51-53, 60-62, 70-72, 80-82, 89-91
openrag/components/indexer/loaders/pdf_loaders/docling2.py (1)
1-5: Import reordering only—looks good.No functional changes spotted in this hunk.
openrag/components/llm.py (3)
39-43: LGTM!The error message formatting consolidation is clean and consistent. The single-line f-string approach improves readability while preserving the same error information (status code and detail).
61-64: LGTM!Consistent formatting with the other error handling paths in this file.
83-85: LGTM!Error formatting is now consistent across all three API error paths in this class.
openrag/routers/users.py (2)
13-14: LGTM!The multi-line decorator formatting with the path on a separate line improves readability and is consistent with Ruff's formatting preferences.
Also applies to: 37-38, 60-61, 102-103, 133-134, 169-170
83-89: LGTM!The type hints are correctly modernized to Python 3.10+ union syntax (
str | None). TheForm(None)default values work correctly with these types.openrag/components/retriever.py (3)
3-3: LGTM!Good use of
ClassVar[dict]annotation for the class-levelRETRIEVERSmapping. This explicitly documents the intent that this is a class variable, not an instance variable.Also applies to: 141-145
36-40: LGTM!The
with_surrounding_chunksparameter is now explicitly declared in the signature rather than being passed through**kwargs. This improves API clarity and IDE support.
78-79: LGTM!The prompt construction is functionally equivalent and the single-line formatting is cleaner.
openrag/scripts/migrations/alembic/versions/4add4d260575_initial_migration.py (1)
9-9: LGTM!Good modernization: using
collections.abc.Sequenceinstead of the deprecatedtyping.Sequence, and adopting Python 3.10+ union syntax for the Alembic revision identifiers. The migration logic remains unchanged.Also applies to: 33-35
openrag/routers/search.py (1)
18-19: LGTM! Type annotations and decorator formatting modernized.The changes correctly update:
- Type hint from
Optional[List[str]]tolist[str] | None(Python 3.10+ syntax)- Decorator formatting to multiline for improved readability
These are consistent with the PR's linting and modernization objectives.
Also applies to: 49-51, 86-87, 137-138
openrag/scripts/migrations/alembic/versions/cd642e4502d8_create_users_memberships_tables.py (1)
9-9: LGTM! Migration metadata type annotations modernized.The changes correctly:
- Import
Sequencefromcollections.abcinstead oftyping(preferred since Python 3.9+)- Update union types to Python 3.10+ syntax (
str | Sequence[str] | None)No functional impact on migration behavior.
Also applies to: 33-35
openrag/components/indexer/chunker/chunker.py (2)
145-145: LGTM! Type annotations modernized throughout the chunker module.Type hints properly updated to Python 3.10+ syntax:
dict | Nonefor optional dictionary parametersstr | Nonefor optional string parametersBaseEmbedding | Nonefor optional embedderThese changes align with the broader PR objective of type modernization.
Also applies to: 161-161, 215-215, 288-288, 347-347
25-28: LGTM! Constants and formatting consolidated.The single-line formatting for
MAX_CONCURRENT_CONTEXTUALIZATIONandBASE_CHUNK_FORMATimproves readability and consistency with the linting objectives.tests/api_tests/test_indexer.py (1)
106-106: LGTM! Test method signatures reformatted.The single-line formatting for test method signatures is consistent with the PR's linting and formatting objectives. No functional changes to test behavior.
Also applies to: 136-136
openrag/routers/actors.py (2)
33-35: LGTM! Decorator formatting improved.The multiline decorator formatting with the path on a separate line improves readability, especially for decorators with long descriptions.
Also applies to: 74-76
125-125: LGTM! Error message formatting modernized.Using
{e!s}instead of{str(e)}is more concise and idiomatic Python. The!sformat specifier callsstr()on the value, producing equivalent output with cleaner syntax.Also applies to: 145-145
openrag/routers/openai.py (1)
194-198: LGTM — consistent exception string formatting.Also applies to: 244-249, 323-326, 338-340
openrag/components/indexer/loaders/pdf_loaders/marker.py (1)
217-221: LGTM — signature aligns with BaseLoader’s 3.10+ typing.openrag/components/indexer/loaders/__init__.py (1)
18-25: LGTM — typing modernization only.Also applies to: 32-40, 54-54
openrag/components/indexer/loaders/txt_loader.py (1)
5-13: LGTM — typing updates are consistent with the loader base API.Also applies to: 26-30, 65-69
openrag/models/indexer.py (1)
5-7: Code syntax is compatible. The project explicitly enforcesrequires-python = ">=3.12"inpyproject.tomlanduv.lock, which exceeds the Python 3.10+ requirement forint | Noneunion syntax. No action needed.openrag/components/indexer/loaders/base.py (5)
13-13: Logger import update looks fine.
36-41: Abstract loader signature is consistent.
66-69: Image description input handling looks good.
103-113: Invalid image data returns a clear sentinel.
133-149: Error logging path is clear.openrag/routers/indexer.py (6)
4-4: No concerns in these small declaration tweaks.Also applies to: 313-313, 358-358
107-107: Docstring tweak noted.
166-172: User context is now propagated to indexing tasks.
282-289: Replacement uploads now preserve user context.
375-375: Copy task now carries the user context.
477-503: Task log retrieval cleanup looks good.openrag/components/indexer/vectordb/utils.py (2)
153-158: Connection error reporting looks good.
181-181: Public method signatures remain consistent.Also applies to: 206-212, 334-339, 377-377, 401-401
openrag/components/indexer/loaders/eml_loader.py (5)
1-19: Imports and helper setup look good.
30-56: Email metadata extraction path is clear.
61-103: Body and attachment parsing flow looks solid.
107-260: Attachment processing and fallbacks look robust.
266-301: Metadata enrichment and markdown output look good.openrag/api.py (7)
4-4: No concerns in these import adjustments.Also applies to: 7-7, 10-10, 31-31
17-22: Verify module-level initialization is safe.This runs on import; please confirm it won’t reinitialize in reloads/tests. If needed, guard initialization.
🛠️ Possible guard
-ray.init(dashboard_host="0.0.0.0") +if not ray.is_initialized(): + ray.init(dashboard_host="0.0.0.0")
37-46: No issues noted here.
73-77: No concerns with these flag declarations.
97-97: No issues noted here.
123-124: No issues noted in these exposure/mounting tweaks.Also applies to: 186-197
237-243: No issues noted in runtime start wiring.Also applies to: 246-246
openrag/components/indexer/loaders/pdf_loaders/openai.py (5)
18-20: LGTM: module-level helper signature is clean.
31-40: Semaphore initialization looks fine.
42-47: Type-hint modernization is consistent.
71-75: Helper signatures updated consistently.
100-107: Verifyimage_urlpayload shape against the target OpenAI‑compatible endpoint.
Some providers accept a string URL, others require an object withurl(and optionallydetail). Please confirm the expected schema for your OCR model before release.openrag/models/openai.py (3)
1-1: No review comment for this change.
36-50: LGTM: completion request fields look consistent.
17-30: No action required; Pydantic v2 handles mutable defaults safely.In Pydantic v2 (currently v2.11.7),
Field({...})with dict literals automatically deep-copies the default for each instance. This prevents shared mutable state across requests, so the current code is safe from cross-request leakage.Using
default_factoryremains a best practice for clarity and explicit intent, but is not necessary for correctness in this case.openrag/components/indexer/vectordb/vectordb.py (15)
56-98: LGTM: abstract method signature updates are consistent.
149-183: Initializer updates look good.
185-243: Collection load/logging/error formatting looks fine.
245-309: Schema/index formatting changes are OK.
335-402: Add-documents flow remains intact.
434-527: Search path updates look consistent.
528-571: Surrounding-chunks dedup logic looks unchanged.
573-602: Delete-file path updates are OK.
603-660: File-chunk retrieval changes look good.
661-702: Chunk-by-id metadata filtering looks fine.
703-736: File existence and partition list wrappers are OK.
750-780: Partition deletion error formatting is consistent.
842-862: List-all-chunk error handling looks consistent.
863-906: User/partition management updates look good.
952-959: File existence check is unchanged in behavior.openrag/components/indexer/indexer.py (12)
7-7: No review comment for this change.
52-55: LGTM: chunk return typing is consistent.
68-118: Add-file flow updates look good.
149-159: Delete-file logging update looks fine.
164-189: Metadata update path is consistent.
192-221: Copy-file changes look good.
223-241: Search signature/default handling looks good.
243-259: Partition validation helpers are OK.
262-268: TaskInfo typing updates look good.
270-275: TaskStateManager state containers are OK.
321-344: Getter return types look consistent.
351-382: Queue info helpers look good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import ast | ||
| import asyncio | ||
| import os | ||
| import httpx | ||
| import json | ||
| import os | ||
| import random | ||
| import time | ||
| import numpy as np | ||
| import ast | ||
|
|
||
| import hdbscan | ||
| import httpx | ||
| import numpy as np | ||
| from dotenv import load_dotenv | ||
| from loguru import logger | ||
| from langchain_openai import ChatOpenAI | ||
| from tqdm.asyncio import tqdm | ||
| from loguru import logger | ||
| from sklearn.cluster import DBSCAN | ||
| import umap.umap_ as umap | ||
| import hdbscan | ||
| import pickle | ||
| from tqdm.asyncio import tqdm |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find dependency manifest files
echo "=== Locating dependency manifests ==="
manifest_files=$(fd -a -t f \( -name 'pyproject.toml' -o -name 'requirements*.txt' -o -name 'setup.cfg' -o -name 'setup.py' -o -name 'Pipfile' -o -name 'poetry.lock' \) 2>/dev/null || echo "")
if [ -z "$manifest_files" ]; then
echo "No dependency manifest files found."
exit 0
fi
echo "$manifest_files"
echo
# Check each manifest for the new dependencies
echo "=== Checking for new dependencies: hdbscan, httpx, numpy, tqdm ==="
echo
for file in $manifest_files; do
echo "--- File: $file ---"
if grep -E '(hdbscan|httpx|numpy|tqdm)' "$file" 2>/dev/null || echo "Not found in this file"; then
echo
fi
done
# Also check if there's an automatic-evaluation-pipeline directory structure
echo "=== Directory structure check ==="
if [ -d "automatic-evaluation-pipeline" ]; then
echo "Found automatic-evaluation-pipeline directory"
ls -la automatic-evaluation-pipeline/ | head -20
fiRepository: linagora/openrag
Length of output: 133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Repository root structure ==="
ls -la | head -30
echo
echo "=== All Python dependency-related files in the entire repo ==="
find . -type f \( -name '*requirements*' -o -name 'pyproject.toml' -o -name 'setup.py' -o -name 'setup.cfg' -o -name 'Pipfile' -o -name 'poetry.lock' -o -name 'Pipfile.lock' \) 2>/dev/null | head -20
echo
echo "=== Locate automatic-evaluation-pipeline directory ==="
find . -type d -name 'automatic-evaluation-pipeline' 2>/dev/null
echo
echo "=== Check for generate_questions.py file ==="
find . -name 'generate_questions.py' 2>/dev/null
echo
echo "=== List Python files in automatic-evaluation-pipeline (if exists) ==="
if [ -d "automatic-evaluation-pipeline" ]; then
ls -la automatic-evaluation-pipeline/
fiRepository: linagora/openrag
Length of output: 2786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Content of pyproject.toml ==="
cat ./pyproject.toml
echo
echo "=== Content of utility/requirements.txt ==="
cat ./utility/requirements.txt
echo
echo "=== Searching for new dependencies in all files ==="
echo "Looking for: hdbscan, httpx, numpy, tqdm"
rg -i "(hdbscan|httpx|numpy|tqdm)" ./pyproject.toml ./utility/requirements.txt || echo "Pattern not found"Repository: linagora/openrag
Length of output: 2675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking imports in generate_questions.py ==="
head -20 ./automatic-evaluation-pipeline/generate_questions.py
echo
echo "=== Searching for numpy and tqdm usage in the file ==="
rg -i "numpy|tqdm" ./automatic-evaluation-pipeline/generate_questions.py | head -20Repository: linagora/openrag
Length of output: 659
Add missing runtime dependencies numpy and tqdm to pyproject.toml.
The imports numpy and tqdm are not declared in the project's dependency manifest. Code will fail at runtime with ModuleNotFoundError. Add them to the dependencies list in pyproject.toml with version constraints (e.g., numpy>=1.24.0, tqdm>=4.66.0). Note: hdbscan>=0.8.40 and httpx are already declared.
🤖 Prompt for AI Agents
In `@automatic-evaluation-pipeline/generate_questions.py` around lines 1 - 15, The
project is missing runtime dependencies for the imports "numpy" and "tqdm" used
in generate_questions.py; open the project's pyproject.toml and add entries for
numpy (e.g., numpy>=1.24.0) and tqdm (e.g., tqdm>=4.66.0) to the top-level
[project.dependencies] (or equivalent dependencies list) alongside the existing
hdbscan and httpx entries so the imports in generate_questions.py (numpy,
tqdm.asyncio.tqdm) resolve at runtime.
| async def serialize_file(task_id: str, path: str, metadata: dict | None = None): | ||
| import ray | ||
|
|
||
| from components.ray_utils import call_ray_actor_with_timeout | ||
|
|
||
| metadata = metadata or {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the specific file and lines mentioned
cat -n openrag/components/indexer/utils/files.py | sed -n '73,80p'Repository: linagora/openrag
Length of output: 461
🏁 Script executed:
# Check if indexer.py exists and has the mentioned serialize_file method
if [ -f openrag/components/indexer/indexer.py ]; then
cat -n openrag/components/indexer/indexer.py | sed -n '57,70p'
else
echo "File not found"
fiRepository: linagora/openrag
Length of output: 498
🏁 Script executed:
# Search for mutable default arguments pattern with metadata
rg -n --type=py 'def.*metadata.*=\s*\{\}' openrag/Repository: linagora/openrag
Length of output: 42
🏁 Script executed:
# More general search for all mutable default arguments in python files
rg -n --type=py 'def [^(]*\([^)]*=\s*\{\}' openrag/ | head -20Repository: linagora/openrag
Length of output: 42
Apply the same mutable default argument fix to indexer.py.
The fix in files.py is correct: changing metadata: dict | None = {} to metadata: dict | None = None with normalization metadata = metadata or {} properly addresses the mutable default argument anti-pattern.
However, the serialize_file method in openrag/components/indexer/indexer.py at line 61 still has metadata: dict = {}, which exhibits the same mutable default issue. Apply the same fix pattern there for consistency and to prevent shared state bugs.
🤖 Prompt for AI Agents
In `@openrag/components/indexer/utils/files.py` around lines 73 - 77, The
serialize_file function in indexer.py currently uses a mutable default
(metadata: dict = {}); change the parameter to accept None (e.g., metadata: dict
| None = None), then inside serialize_file normalize with metadata = metadata or
{} (or explicit if metadata is None: metadata = {}); update any type
hints/usages accordingly to avoid shared mutable state and mirror the fix
applied in files.py.
| try: | ||
| files = pfm.list_partition_files(part_name) | ||
| except Exception as e: | ||
| logger.error(f'Failed while requesting the list of files in partition \'{part_name}\'\n{e}') | ||
| logger.error( | ||
| f"Failed while requesting the list of files in partition '{part_name}'\n{e}" | ||
| ) | ||
| raise | ||
|
|
||
| files['files'].sort(key=lambda v: v['file_id']) | ||
| files["files"].sort(key=lambda v: v["file_id"]) | ||
|
|
||
| for f in files['files']: | ||
| f.pop('partition', None) | ||
| out_fh.write(json.dumps(f, ensure_ascii=False, sort_keys=True) + '\n') | ||
| for f in files["files"]: | ||
| f.pop("partition", None) | ||
| out_fh.write(json.dumps(f, ensure_ascii=False, sort_keys=True) + "\n") | ||
|
|
||
| # Separator | ||
| out_fh.write('\n') | ||
| out_fh.write("\n") | ||
|
|
||
| if verbose: | ||
| logger.info(f'Partition \'{part_name}\' - {len(files["files"])} files') | ||
| logger.info(f"Partition '{part_name}' - {len(files['files'])} files") |
There was a problem hiding this comment.
Handle empty partition file lists to avoid KeyError.
PartitionFileManager.list_partition_files() returns {} when a partition has no files; files["files"] will crash the backup. Default to an empty list and use that consistently.
🐛 Proposed fix
- files["files"].sort(key=lambda v: v["file_id"])
-
- for f in files["files"]:
+ file_list = files.get("files", [])
+ file_list.sort(key=lambda v: v["file_id"])
+
+ for f in file_list:
f.pop("partition", None)
out_fh.write(json.dumps(f, ensure_ascii=False, sort_keys=True) + "\n")
@@
- logger.info(f"Partition '{part_name}' - {len(files['files'])} files")
+ logger.info(f"Partition '{part_name}' - {len(file_list)} files")🤖 Prompt for AI Agents
In `@openrag/scripts/backup.py` around lines 51 - 69,
PartitionFileManager.list_partition_files() may return an empty dict causing
files["files"] KeyError; change the handling after calling
pfm.list_partition_files(part_name) to default files_list = files.get("files",
[]) (or similar) and use files_list for sorting, popping "partition", writing
JSON lines to out_fh, writing the separator, and logging the count (use
len(files_list)); keep the rest of the try/except and variable names
(pfm.list_partition_files, files, out_fh, verbose, logger) intact.
2 participants
🎨 Add Ruff Linting, Modernize Type Annotations, and Improve Code Quality
This PR introduces automated code quality tooling and modernizes the codebase to align with contemporary Python standards and best practices.
🚀 What's New
Automated Linting & CI Integration
.github/workflows/lint.ymlto automatically run Ruff linting and formatting checks on every push tomainanddevbranches, ensuring consistent code quality across all contributionsLINTING.mdguide covering local setup, pre-commit hooks, CI integration, and troubleshooting tipsType Annotation Modernization
Migrated type hints to modern Python 3.10+ syntax throughout the codebase:
Union[str, List[str]]withstr | list[str]Optional[List]withlist | None.github/workflows/api_tests/mock_vllm.pyfor improved readability and maintainability✅ Testing
All existing tests continue to pass. The new linting workflow validates code quality on every push.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.