Feat/chunking - #165
Conversation
b249d15 to
d890071
Compare
WalkthroughRefactors chunking to a single RecursiveSplitter with page-aware MDElement representations, adds chunk contextualization (language-aware), supports surrounding-chunk retrieval, introduces token-aware context formatting, increases embedder max_model_len to 8192, and updates configs, docs, and tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Pipeline as RAG Pipeline
participant Chunker
participant Ctx as ChunkContextualizer
participant VectorDB
participant Retriever
Client->>Pipeline: submit document / request
Pipeline->>Chunker: split_document(content)
Chunker->>Chunker: _prepare_md_elements → MDElement list
Chunker->>Chunker: split_text / chunk_table → chunks
Chunker->>Ctx: detect_language(chunk_text)
Ctx-->>Chunker: language
Chunker->>Ctx: generate_context(chunk, lang)
Ctx-->>Chunker: contextualized_chunk
Chunker-->>Pipeline: list[Document] (with metadata)
Pipeline->>VectorDB: async_add_documents(chunks)
VectorDB->>VectorDB: attach order metadata & index
Client->>Pipeline: retrieve(query)
Pipeline->>Retriever: retrieve(query)
Retriever->>VectorDB: async_search(query, with_surrounding_chunks=True)
VectorDB->>VectorDB: search & get_surrounding_chunks(results)
VectorDB-->>Retriever: list[Document] (including surrounding)
Retriever-->>Pipeline: retrieved_docs
Pipeline->>Pipeline: format_context(docs, max_context_tokens)
Pipeline-->>Client: formatted_context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d890071 to
3833bd9
Compare
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
3833bd9 to
1832da8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1832da8 to
5e5b206
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openrag/components/indexer/loaders/base.py (1)
127-130: Align error message formatting with the updated image description format.Lines 127 and 130 return error messages wrapped in
<image_description>tags using single newlines, while line 153 now uses double newlines. This inconsistency may cause downstream parsing or chunking logic to handle error cases differently than successful descriptions.Apply this diff to align the formatting:
- return """\n<image_description>\nInvalid image data format\n</image_description>\n""" + return """<image_description>\n\nInvalid image data format\n\n</image_description>"""- return """\n<image_description>\nUnsupported image data type\n</image_description>\n""" + return """<image_description>\n\nUnsupported image data type\n\n</image_description>"""openrag/components/indexer/chunker/chunker.py (1)
327-331: Dead code:semantic_splitteris not registered inCHUNKERS.This code block is unreachable because
semantic_splitteris not in theCHUNKERSdictionary. The factory raisesValueErrorat line 320-324 before reaching this block. Either remove this dead code or register the semantic splitter inCHUNKERS.- # Add embeddings if semantic splitter is selected - if name == "semantic_splitter": - embedder = EmbeddingFactory.get_embedder( - embeddings_config=dict(config.embedder) - ) - chunker_params["embeddings"] = embedder - chunker_params["llm_config"] = config.vlm return chunker_cls(**chunker_params)
🧹 Nitpick comments (11)
prompts/example1/chunk_contextualizer_tmpl.txt (1)
1-4: Tighten wording and align terminology in the chunk contextualizer promptYou could make this a bit clearer and more consistent with the later “Previous Chunks”/“Current Chunk” labels:
- Line 4: avoid “with respect to” and align with plural “Previous Chunks”, e.g.:
-Write a concise standalone sentence that summarizes the context of current chunk with respect to the previous chunk using the following context +Write a concise standalone sentence that summarizes the context of the current chunk and how it relates to the previous chunks, using the following information:
- Line 23: align with other bullets by adding a colon:
-- Current Chunk +- Current Chunk:Also applies to: 23-24
openrag/components/utils.py (1)
124-146: Instantiating ChatOpenAI on every call is inefficient.
ChatOpenAI(**config.llm)creates a new client instance each timeformat_contextis called. This involves connection setup overhead. Consider caching the LLM instance at module level or using a lazy singleton pattern.+# Module-level cached LLM for tokenization +_tokenizer_llm = None + +def _get_tokenizer_llm(): + global _tokenizer_llm + if _tokenizer_llm is None: + _tokenizer_llm = ChatOpenAI(**config.llm) + return _tokenizer_llm + + def format_context(docs: list[Document], max_context_tokens: int = 4096) -> str: if not docs: return "No document found from the database" - llm = ChatOpenAI(**config.llm) + llm = _get_tokenizer_llm() _length_function = llm.get_num_tokensopenrag/components/indexer/chunker/utils.py (3)
23-23: Use explicitOptionaltype hint for nullable parameter.PEP 484 prohibits implicit
Optional. Update the type hint forpage_number.def __init__( self, type: Literal["text", "table", "image"], content: str, - page_number: int = None, + page_number: int | None = None, ):
223-225: Use explicitOptionaltype hint forlength_function.def chunk_table( - table_element: MDElement, chunk_size: int = 512, length_function: callable = None + table_element: MDElement, chunk_size: int = 512, length_function: callable | None = None ) -> list[MDElement]:
94-94: Text segments lack page number assignment.Text segments are appended as
("text", text_segment.strip())without a page number, while tables and images includepage_num. This inconsistency means textMDElementobjects will havepage_number=Nonewhile tables/images have actual page numbers. Consider assigning page numbers to text segments as well for consistency.if start > last: text_segment = md_text[last:start] if text_segment.strip(): # Only add non-empty text segments - parts.append(("text", text_segment.strip())) + text_page_num = get_page_number(last, page_markers) + parts.append(("text", text_segment.strip(), text_page_num))Apply similar change for the remaining text at line 104:
remaining_text = md_text[last:] if remaining_text.strip(): # Only add non-empty text segments - parts.append(("text", remaining_text.strip())) + remaining_page_num = get_page_number(last, page_markers) + parts.append(("text", remaining_text.strip(), remaining_page_num))openrag/components/indexer/vectordb/vectordb.py (2)
387-388: Addstrict=Truetozip()to catch length mismatches.While the lengths should match by construction, using
strict=Trueprovides a safety check and documents the expectation.order_metadata_l: list[dict] = _gen_chunk_order_metadata(n=len(chunks)) - for chunk, vector, order_metadata in zip(chunks, vectors, order_metadata_l): + for chunk, vector, order_metadata in zip(chunks, vectors, order_metadata_l, strict=True):
556-556: Typo:existant_idsshould beexisting_ids.- existant_ids = set(doc.metadata.get("_id") for doc in docs) + existing_ids = set(doc.metadata.get("_id") for doc in docs)And update subsequent usages on lines 581-582.
openrag/components/indexer/chunker/chunker.py (4)
43-44: Re-raise exception with context usingfrom e.This preserves the exception chain for debugging.
except Exception as e: - raise ValueError(f"Error creating context generator: {e}") + raise ValueError(f"Error creating context generator: {e}") from e
118-118:BaseChunkerinherits fromABCbut has no abstract methods.Either add
@abstractmethoddecorators to methods that subclasses must implement, or remove theABCinheritance since all methods have concrete implementations.-class BaseChunker(ABC): +class BaseChunker: """Base class for document chunkers with built-in contextualization capability."""
70-72: Remove unusedmetadataparameter.The
metadataparameter is not used in this method.async def contextualize_chunks( - self, chunks: list[Document], metadata: dict = None + self, chunks: list[Document] ) -> list[Document]:Also update the call site at line 152:
- return await self.contextualizer.contextualize_chunks(chunks, metadata=metadata) + return await self.contextualizer.contextualize_chunks(chunks)
145-147: Use explicitOptionaltype hints.async def _apply_contextualization( - self, chunks: list[Document], metadata: dict = None + self, chunks: list[Document], metadata: dict | None = None ) -> list[Document]:Similarly at line 196:
def _get_chunks( - self, content: str, metadata: dict = None, log=None + self, content: str, metadata: dict | None = None, log=None ) -> list[Document]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.github/workflows/smoke_test/.env(1 hunks).hydra_config/chunker/semantic_splitter.yaml(0 hunks).hydra_config/chunker/token_splitter.yaml(0 hunks).hydra_config/config.yaml(3 hunks).hydra_config/rag/ChatBotRag.yaml(1 hunks).hydra_config/rag/SimpleRag.yaml(1 hunks).hydra_config/rag/base.yaml(1 hunks).hydra_config/retriever/base.yaml(1 hunks)docker-compose.yaml(2 hunks)docs/content/docs/documentation/env_vars.md(3 hunks)openrag/components/indexer/chunker/chunker.py(2 hunks)openrag/components/indexer/chunker/utils.py(2 hunks)openrag/components/indexer/embeddings/openai.py(2 hunks)openrag/components/indexer/loaders/base.py(1 hunks)openrag/components/indexer/vectordb/vectordb.py(11 hunks)openrag/components/pipeline.py(3 hunks)openrag/components/prompts/prompts.py(1 hunks)openrag/components/retriever.py(4 hunks)openrag/components/utils.py(3 hunks)prompts/example1/chunk_contextualizer_tmpl.txt(1 hunks)quick_start/docker-compose.yaml(2 hunks)
💤 Files with no reviewable changes (2)
- .hydra_config/chunker/semantic_splitter.yaml
- .hydra_config/chunker/token_splitter.yaml
🧰 Additional context used
🧬 Code graph analysis (4)
openrag/components/pipeline.py (1)
openrag/components/utils.py (1)
format_context(124-146)
openrag/components/utils.py (2)
openrag/utils/logger.py (1)
get_logger(10-47)openrag/config/config.py (1)
load_config(12-29)
openrag/components/indexer/chunker/chunker.py (1)
openrag/components/indexer/chunker/utils.py (5)
MDElement(16-30)chunk_table(187-220)chunk_table(223-279)get_chunk_page_number(109-141)split_md_elements(52-106)
openrag/components/indexer/vectordb/vectordb.py (1)
openrag/components/indexer/indexer.py (1)
chunk(53-57)
🪛 dotenv-linter (4.0.0)
.github/workflows/smoke_test/.env
[warning] 29-29: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🪛 LanguageTool
docs/content/docs/documentation/env_vars.md
[grammar] ~123-~123: Ensure spelling is correct
Context: ...k exceeds this limit, the embedder will trucate it.| If you prefer to use an **externa...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~123-~123: Ensure spelling is correct
Context: ...his limit, the embedder will trucate it.| If you prefer to use an **external embed...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
prompts/example1/chunk_contextualizer_tmpl.txt
[style] ~1-~1: Consider using “who” when you are referring to a person instead of an object.
Context: You are an AI assistant that creates brief contextual summaries to m...
(THAT_WHO)
[style] ~4-~4: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...summarizes the context of current chunk with respect to the previous chunk using the following ...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
🪛 markdownlint-cli2 (0.18.1)
docs/content/docs/documentation/env_vars.md
218-218: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.8)
openrag/components/indexer/chunker/chunker.py
43-43: Do not catch blind exception: Exception
(BLE001)
44-44: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
64-64: Do not catch blind exception: Exception
(BLE001)
71-71: Unused method argument: metadata
(ARG002)
71-71: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
110-110: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
113-113: Do not catch blind exception: Exception
(BLE001)
118-118: BaseChunker is an abstract base class, but it has no abstract methods or properties
(B024)
127-127: Unused method argument: kwargs
(ARG002)
146-146: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
196-196: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
openrag/components/indexer/chunker/utils.py
23-23: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
188-188: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
199-199: Loop control variable groups_ntoks overrides iterable it iterates
(B020)
199-199: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
223-223: Redefinition of unused chunk_table from line 187
(F811)
224-224: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
245-245: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
openrag/components/indexer/vectordb/vectordb.py
388-388: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
532-532: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (28)
openrag/components/indexer/embeddings/openai.py (1)
13-18: max_model_len plumbing looks good; confirm backend support fortruncate_prompt_tokensThe new
max_model_lenfield and its propagation viaextra_body={"truncate_prompt_tokens": self.max_model_len}are consistent and align with the documented default of 8192. Just ensure your OpenAI‑compatible embedder actually honorstruncate_prompt_tokensfor embeddings; on some providers this field may be ignored or use a different name, which would defeat the intended truncation.Also applies to: 37-41
.hydra_config/retriever/base.yaml (1)
3-4: Retriever config additions are consistent with env/docs
similarity_thresholdand the newwith_surrounding_chunksflag are wired in a standard Hydra/oc.env pattern and match the documented environment variables and behavior.docs/content/docs/documentation/env_vars.md (1)
192-196: Retriever env var docs look coherent with configThe new entries for
RETRIEVER_TYPEandWITH_SURROUNDING_CHUNKSclearly describe the available strategies and surrounding‑chunk behavior, and they match the retriever/base Hydra config and PR intent.docker-compose.yaml (1)
47-47: Aligned--max-model-lendefaults with documented 8192 limitBoth GPU and CPU vLLM commands now default
--max-model-lento${MAX_MODEL_LEN:-8192}, matching the new env var and documentation. This keeps docker defaults consistent with the embedder config.Also applies to: 146-146
quick_start/docker-compose.yaml (1)
45-45: Quick-start compose now matches global max-model-len defaultsThe quick-start VLLM templates use
--max-model-len ${MAX_MODEL_LEN:-8192}for both GPU and CPU, keeping behavior aligned with the maindocker-compose.yamland the documentedMAX_MODEL_LEN.Also applies to: 142-142
.hydra_config/rag/SimpleRag.yaml (1)
1-3: Hydradefaultsinheritance for SimpleRag looks correctAdding:
defaults: - base mode: SimpleRagis a standard Hydra pattern to inherit common RAG settings from
basewhile keeping a SimpleRag mode. This should simplify config reuse without changing behavior..hydra_config/rag/base.yaml (1)
1-4: LGTM!Clean extraction of shared RAG configuration defaults. The values (
chat_history_depth: 4,max_contextualized_query_len: 512) provide sensible defaults that child configurations can inherit and override..hydra_config/rag/ChatBotRag.yaml (1)
1-3: LGTM!Good use of Hydra's defaults mechanism to inherit from
base.yaml, reducing configuration duplication while maintaining explicit mode specification..hydra_config/config.yaml (3)
3-3: LGTM!Consolidating to
recursive_splitteras the default chunker aligns with the PR's chunking strategy improvements.
34-34: LGTM!New
max_model_lenparameter with sensible default of 8192 tokens. Properly usesoc.decodefor type conversion from environment variable.
53-53: LGTM!Increasing
top_kfrom 5 to 10 provides more documents for reranking, which complements the new surrounding chunks feature. The inline comment helpfully explains the rationale.openrag/components/pipeline.py (3)
74-79: Calculation assumes uniform chunk sizes.The formula
top_k * chunk_sizeprovides a reasonable upper-bound estimate, but note that with the new chunking strategy (tables, images), actual chunk sizes may vary significantly from the configuredchunk_size. Theformat_contextfunction will handle overflow by truncation, so this isn't a correctness issue, but context utilization could be suboptimal.Consider whether this heuristic adequately represents the actual token budget when chunks have variable sizes. If chunks are typically smaller than
chunk_size, you may be under-utilizing the context window.
152-152: LGTM!Properly passes
max_context_tokenstoformat_context, enabling token-aware context construction.
183-183: LGTM!Consistent with the chat completion path, properly passing
max_context_tokensfor completions.openrag/components/retriever.py (4)
35-41: LGTM!Clean addition of
with_surrounding_chunksparameter with sensible default. Properly stored as instance attribute for use in retrieval calls.
54-54: LGTM!Correctly propagates the
with_surrounding_chunksflag to the vector database search.
100-100: LGTM!Consistent propagation of
with_surrounding_chunksin MultiQueryRetriever.
141-141: LGTM!Consistent propagation of
with_surrounding_chunksin HyDeRetriever.openrag/components/utils.py (2)
9-14: LGTM!Appropriate imports and logger initialization for the new token-aware functionality.
163-164: Module-level semaphore initialization is intentional.These calls ensure the distributed semaphore actors are created at import time. This is a valid pattern for eager initialization of Ray actors.
openrag/components/prompts/prompts.py (1)
33-33: All usages of the renamed variable have been properly updated. The old nameCHUNK_CONTEXTUALIZERhas no remaining references in the codebase, and the new nameCHUNK_CONTEXTUALIZER_PMPTis correctly imported and used inopenrag/components/indexer/chunker/chunker.py.openrag/components/indexer/chunker/utils.py (2)
109-141: LGTM!The page number detection logic correctly handles edge cases for chunks with page markers.
282-306: LGTM!The table spacing normalization handles edge cases properly.
openrag/components/indexer/vectordb/vectordb.py (2)
523-532: LGTM!The surrounding chunks retrieval and result extension logic is correct.
129-131: LGTM!Stop words correctly exclude the new chunk formatting markers from being indexed.
openrag/components/indexer/chunker/chunker.py (3)
195-253: LGTM!The chunk assembly logic correctly handles text splitting, table chunking, and page metadata assignment. The final sort by page ensures proper document order.
279-300: LGTM!The
RecursiveSplittercorrectly initializes the text splitter with regex separators for paragraph and sentence-level splitting.
255-276: LGTM!The document splitting flow is clean and well-logged.
5e5b206 to
929edb2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
929edb2 to
6e48371
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
6e48371 to
24d7156
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
24d7156 to
0070e31
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
0070e31 to
9139418
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
9139418 to
c7c31a8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai generate docstrings |
Docstrings generation was requested by @Ahmath-Gadji. * #165 (comment) The following files were modified: * `openrag/components/indexer/chunker/chunker.py` * `openrag/components/indexer/chunker/test_chunking.py` * `openrag/components/indexer/chunker/utils.py` * `openrag/components/indexer/embeddings/openai.py` * `openrag/components/indexer/loaders/base.py` * `openrag/components/indexer/vectordb/vectordb.py` * `openrag/components/pipeline.py` * `openrag/components/retriever.py` * `openrag/components/utils.py`
… each matched document to provide additional context.
d6adc9f to
b7326d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openrag/components/prompts/prompts.py (1)
11-27: Type hint mismatch: function returnsstr, nottuple[str, str].The function signature declares a return type of
tuple[str, str], but line 27 returns onlysys_msg(a single string).def load_prompt( prompt_name: str, prompts_dir: Path = prompts_dir, prompt_mapping: dict = prompt_mapping, -) -> tuple[str, str]: +) -> str:openrag/components/retriever.py (1)
157-160: Bug: Incorrect variable checked in validation.Line 159 checks
retriever_type is None, but the intent is to validate whether the retriever class was found. This should checkretriever_cls is Noneinstead. As written, an unknown retriever type silently returnsNonefromRETRIEVERS.get()and will raise anAttributeErrorlater when instantiating.retriever_cls = RetrieverFactory.RETRIEVERS.get(retriever_type, None) - if retriever_type is None: + if retriever_cls is None: raise ValueError(f"Unknown retriever type: {retriever_type}")
♻️ Duplicate comments (3)
openrag/components/indexer/vectordb/vectordb.py (1)
555-600: Add guard for empty query responses to prevent IndexError.As noted in the past review, if a query returns an empty response (e.g., chunk deleted or section_id doesn't exist), accessing
response[0]on line 586 will raise anIndexError.Apply this diff to add a guard:
# Build output, skipping duplicates output_docs = [] for response in responses: + if not response: + continue doc_id = response[0].get("_id") if doc_id not in existant_ids:Based on past review comments.
openrag/components/utils.py (2)
125-147: Cache the tokenizer to avoid recreatingChatOpenAIon every call.
ChatOpenAI(**config.llm)is instantiated on every call toformat_context. Since onlyget_num_tokensis needed, cache it at module scope to improve performance.config = load_config() logger = get_logger() +_token_counter = None + +def _get_token_counter(): + global _token_counter + if _token_counter is None: + _token_counter = ChatOpenAI(**config.llm).get_num_tokens + return _token_counter def format_context(docs: list[Document], max_context_tokens: int = 4096) -> str: if not docs: return "No document found from the database" - llm = ChatOpenAI(**config.llm) - _length_function = llm.get_num_tokens + _length_function = _get_token_counter() docs_with_tokens = list(map(lambda d: (_length_function(d.page_content), d), docs))
150-157: Fix import-time PermissionError: use configurable cache directory with lazy initialization.The hardcoded path
/app/model_weights/at line 151 will cause aPermissionErrorduring import in non-Docker environments (e.g., pytest runs). The directory creation happens at module load time, which breaks portability.Use lazy initialization with a configurable cache directory:
-# Initialize language detector -lang_detect_cache_dir = "/app/model_weights/" -lang_detector_config = LangDetectConfig( - max_input_length=1024, # chars - model="auto", - cache_dir=lang_detect_cache_dir, -) -lang_detector: LangDetector = LangDetector(config=lang_detector_config) +# Lazy initialization of language detector +_lang_detector: LangDetector = None + +def _get_lang_detector(): + global _lang_detector + if _lang_detector is None: + import os + cache_dir = os.getenv("LANG_DETECT_CACHE_DIR", os.path.expanduser("~/.cache/openrag/lang_detect")) + os.makedirs(cache_dir, exist_ok=True) + _lang_detector = LangDetector(config=LangDetectConfig( + max_input_length=1024, + model="auto", + cache_dir=cache_dir, + )) + return _lang_detectorThen update
detect_language:def detect_language(text: str): - outputs = lang_detector.detect(text, k=1) + outputs = _get_lang_detector().detect(text, k=1) return outputs[0].get("lang")
🧹 Nitpick comments (6)
openrag/components/indexer/vectordb/vectordb.py (1)
387-396: Addstrict=Trueto zip for runtime safety.As noted in the past review and static analysis, adding
strict=Trueto the zip call ensures length mismatches are caught at runtime rather than silently dropping data.Apply this diff:
- for chunk, vector, order_metadata in zip(chunks, vectors, order_metadata_l): + for chunk, vector, order_metadata in zip(chunks, vectors, order_metadata_l, strict=True):Based on past review and static analysis hints.
pyproject.toml (1)
47-49: Consolidate language detection to usefast_langdetectconsistently.Both
langdetect>=1.0.9andfast_langdetect>=1.0.0are imported and used for the same purpose. Whilefast_langdetectis used in the centralizeddetect_language()function (utils.py, used by chunker.py), the legacylangdetectis used separately inmedia_loader.py's_detect_language()method. Refactor media_loader to use the centralized language detection or adoptfast_langdetectdirectly to avoid duplicate dependencies and align with the faster implementation.openrag/components/retriever.py (1)
59-60: Typo in class name:SingleRetreiver→SingleRetriever.-class SingleRetreiver(BaseRetriever): +class SingleRetriever(BaseRetriever): passopenrag/components/indexer/chunker/utils.py (1)
17-31: Consider renamingtypeparameter to avoid shadowing built-in.Using
typeas a parameter/attribute name shadows Python's built-intype()function. While functional, this can cause subtle issues iftype()is needed within the class.class MDElement: """Class representing a segment of markdown content.""" def __init__( self, - type: Literal["text", "table", "image"], + element_type: Literal["text", "table", "image"], content: str, page_number: Optional[int] = None, ): - self.type = type # 'text', 'table', 'image' + self.type = element_type # 'text', 'table', 'image' self.content = content self.page_number = page_numberopenrag/components/indexer/chunker/chunker.py (2)
109-134: Consider adding an abstract method or removing ABC inheritance.
BaseChunkerinherits fromABCbut has no abstract methods, making the ABC inheritance unnecessary. Either add an abstract method (e.g.,split_text) or remove the ABC inheritance.-class BaseChunker(ABC): +class BaseChunker: """Base class for document chunkers with built-in contextualization capability."""
63-67: Consider logging exception details for debugging.The broad
Exceptioncatches are acceptable for resilience, but logging only the exception message may lose stack trace information useful for debugging. Consider usinglogger.warning(..., exc_info=True)orlogger.exception(...)for better diagnostics.Also applies to: 104-106
📜 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 (25)
.github/workflows/smoke_test/.env(1 hunks).hydra_config/chunker/semantic_splitter.yaml(0 hunks).hydra_config/chunker/token_splitter.yaml(0 hunks).hydra_config/config.yaml(3 hunks).hydra_config/rag/ChatBotRag.yaml(1 hunks).hydra_config/rag/SimpleRag.yaml(1 hunks).hydra_config/rag/base.yaml(1 hunks).hydra_config/retriever/base.yaml(1 hunks)docker-compose.yaml(2 hunks)docs/content/docs/documentation/env_vars.md(4 hunks)openrag/components/indexer/chunker/chunker.py(2 hunks)openrag/components/indexer/chunker/test_chunking.py(1 hunks)openrag/components/indexer/chunker/utils.py(1 hunks)openrag/components/indexer/embeddings/openai.py(2 hunks)openrag/components/indexer/loaders/base.py(1 hunks)openrag/components/indexer/vectordb/vectordb.py(11 hunks)openrag/components/pipeline.py(3 hunks)openrag/components/prompts/prompts.py(1 hunks)openrag/components/retriever.py(4 hunks)openrag/components/utils.py(3 hunks)prompts/example1/chunk_contextualizer_tmpl.txt(1 hunks)prompts/example1/sys_prompt_tmpl.txt(1 hunks)pyproject.toml(1 hunks)pytest.ini(1 hunks)quick_start/docker-compose.yaml(2 hunks)
💤 Files with no reviewable changes (2)
- .hydra_config/chunker/token_splitter.yaml
- .hydra_config/chunker/semantic_splitter.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- openrag/components/indexer/embeddings/openai.py
- .hydra_config/rag/base.yaml
- openrag/components/indexer/chunker/test_chunking.py
- .hydra_config/config.yaml
- .hydra_config/rag/ChatBotRag.yaml
- openrag/components/indexer/loaders/base.py
- openrag/components/pipeline.py
- docs/content/docs/documentation/env_vars.md
- pytest.ini
- .hydra_config/rag/SimpleRag.yaml
🧰 Additional context used
🧬 Code graph analysis (3)
openrag/components/utils.py (2)
openrag/utils/logger.py (1)
get_logger(10-47)openrag/config/config.py (1)
load_config(12-29)
openrag/components/indexer/vectordb/vectordb.py (1)
openrag/components/indexer/indexer.py (1)
chunk(53-57)
openrag/components/indexer/chunker/chunker.py (6)
openrag/components/utils.py (2)
detect_language(160-162)get_vlm_semaphore(172-176)openrag/config/config.py (1)
load_config(12-29)openrag/utils/logger.py (1)
get_logger(10-47)openrag/components/indexer/embeddings/base.py (1)
BaseEmbedding(5-20)openrag/components/indexer/chunker/utils.py (3)
MDElement(17-31)get_chunk_page_number(110-145)split_md_elements(53-107)openrag/components/indexer/indexer.py (1)
chunk(53-57)
🪛 Ruff (0.14.8)
openrag/components/indexer/vectordb/vectordb.py
388-388: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
532-532: Consider moving this statement to an else block
(TRY300)
openrag/components/indexer/chunker/chunker.py
62-62: Consider moving this statement to an else block
(TRY300)
63-63: Do not catch blind exception: Exception
(BLE001)
104-104: Do not catch blind exception: Exception
(BLE001)
109-109: BaseChunker is an abstract base class, but it has no abstract methods or properties
(B024)
118-118: Unused method argument: kwargs
(ARG002)
⏰ 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
🔇 Additional comments (21)
quick_start/docker-compose.yaml (1)
45-45: LGTM: Max model length standardized to 8192.The default max-model-len value has been consistently updated to 8192 across both the x-vllm template and vllm-cpu service, aligning with the embedder model capabilities documented elsewhere in the PR.
Also applies to: 142-142
.hydra_config/retriever/base.yaml (1)
4-4: LGTM: Surrounding chunks configuration added.The new
with_surrounding_chunksparameter (default:true) integrates cleanly with the retriever and vector DB changes, enabling optional context expansion during retrieval.docker-compose.yaml (1)
47-47: LGTM: Max model length standardized to 8192.Consistent with quick_start/docker-compose.yaml, the max-model-len is now set to 8192 across both vllm services.
Also applies to: 146-146
.github/workflows/smoke_test/.env (1)
29-30: LGTM: MAX_MODEL_LEN properly configured.The inline comment has been correctly separated from the value assignment, resolving the previous parsing concern. The value is now cleanly set to 8192.
prompts/example1/chunk_contextualizer_tmpl.txt (1)
1-22: LGTM: Contextualization prompt is clear and well-structured.The restructured template provides explicit guidance with clear field definitions, core principles, output format requirements, and practical examples. The emphasis on concise (1-2 sentences), language-aligned context is appropriate for chunking workflows.
prompts/example1/sys_prompt_tmpl.txt (1)
11-11: LGTM: Source citation guideline added.The new rule to avoid citing sources or file names is clear and aligns with the broader prompt restructuring to produce cleaner, more focused responses.
openrag/components/indexer/vectordb/vectordb.py (5)
2-2: LGTM: Standard library import added.The
timeimport is appropriately used for generating deterministic chunk IDs in_gen_chunk_order_metadata.
77-77: LGTM: Surrounding chunks parameter added to abstract interface.The
with_surrounding_chunksparameter is consistently added to both search method signatures with backward-compatible defaults.Also applies to: 89-89
129-131: LGTM: Chunk markers added to stop words.The addition of chunk delimiter markers to the stop words list prevents them from interfering with search indexing.
1020-1035: LGTM: Unique chunk IDs now guaranteed.The
base_ts + ipattern ensures each section_id is unique, addressing the previous time.time_ns() duplication concern.
523-532: LGTM: Conditional surrounding chunks fetching with logging.The logic cleanly extends retrieved documents with surrounding chunks when enabled, with appropriate debug logging.
openrag/components/prompts/prompts.py (1)
33-33: LGTM!The rename from
CHUNK_CONTEXTUALIZERtoCHUNK_CONTEXTUALIZER_PROMPTaligns with the naming convention of other prompt constants in this file (QUERY_CONTEXTUALIZER_PROMPT,HYDE_PROMPT,MULTI_QUERY_PROMPT, etc.).openrag/components/retriever.py (2)
35-41: LGTM!The
with_surrounding_chunksparameter is properly added toBaseRetrieverand correctly propagated through**kwargsto child classes. This enables consistent control of surrounding chunk retrieval across all retriever types.
54-54: Parameter propagation is correct.The
with_surrounding_chunksparameter is consistently passed to the database search methods across all retriever implementations.Also applies to: 100-100, 141-141
openrag/components/indexer/chunker/utils.py (3)
38-50: LGTM!The page number calculation correctly handles the
[PAGE_N]marker semantics where content after the marker belongs to page N+1.
110-145: LGTM!The
get_chunk_page_numberfunction correctly determines start and end pages for chunks containing page separators, handling edge cases like chunks starting/ending exactly at separators.
191-249: LGTM!The table chunking implementation correctly handles overlap by including the previous last row in new subtables, ensuring context continuity across chunks.
openrag/components/indexer/chunker/chunker.py (4)
19-22: LGTM!The chunk format constants provide a consistent structure for contextualized and non-contextualized chunks, including filename metadata for traceability.
55-67: LGTM on semaphore usage.Using
get_vlm_semaphore()for context generation aligns with the use ofconfig.vlmfor the LLM config, ensuring consistent rate limiting.
284-305: LGTM!
RecursiveSplitterproperly initializes the text splitter with paragraph-first, then sentence-level splitting via regex separators. Thestrict=Truein zip (line 101) is a good practice for catching length mismatches.
331-331: The use ofconfig.vlmfor chunker's LLM config is intentional and correct.The chunker processes multi-modal documents containing images, tables, and text. It imports
get_vlm_semaphore()from utils.py, uses it in the context generation method, and explicitly handlesimageandtableelement types during document parsing. The loaders similarly useconfig.vlmfor document processing. In contrast, components likeformat_contextin utils.py, which useconfig.llm, only perform text-only operations like token counting. The distinction is appropriate: VLM config for multi-modal content processing, LLM config for text-only operations.Likely an incorrect or invalid review comment.
b7326d7 to
56dc3bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
openrag/components/indexer/chunker/chunker.py(2 hunks)openrag/components/indexer/chunker/test_chunking.py(1 hunks)openrag/components/indexer/chunker/utils.py(1 hunks)openrag/components/indexer/vectordb/vectordb.py(11 hunks)openrag/components/prompts/prompts.py(2 hunks)openrag/components/retriever.py(5 hunks)pytest.ini(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- openrag/components/indexer/chunker/test_chunking.py
- pytest.ini
🧰 Additional context used
🧬 Code graph analysis (2)
openrag/components/indexer/vectordb/vectordb.py (1)
openrag/components/indexer/indexer.py (1)
chunk(53-57)
openrag/components/indexer/chunker/chunker.py (6)
openrag/components/utils.py (2)
detect_language(160-162)get_vlm_semaphore(172-176)openrag/config/config.py (1)
load_config(12-29)openrag/utils/logger.py (1)
get_logger(10-47)openrag/components/indexer/embeddings/base.py (1)
BaseEmbedding(5-20)openrag/components/indexer/chunker/utils.py (4)
MDElement(17-31)chunk_table(191-253)get_chunk_page_number(110-145)split_md_elements(53-107)openrag/components/indexer/indexer.py (1)
chunk(53-57)
🪛 Ruff (0.14.8)
openrag/components/indexer/vectordb/vectordb.py
388-388: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
532-532: Consider moving this statement to an else block
(TRY300)
openrag/components/indexer/chunker/chunker.py
61-61: Consider moving this statement to an else block
(TRY300)
62-62: Do not catch blind exception: Exception
(BLE001)
103-103: Do not catch blind exception: Exception
(BLE001)
117-117: Unused method argument: kwargs
(ARG002)
⏰ 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
🔇 Additional comments (31)
openrag/components/prompts/prompts.py (2)
11-27: LGTM!The return type change from
tuple[str, str]tostrcorrectly reflects the actual implementation. The function reads and returns a single string from the prompt file.
33-33: LGTM!The constant rename fixes the typo and aligns with the naming convention used by other prompt constants in this file.
openrag/components/retriever.py (6)
35-41: LGTM!The
with_surrounding_chunksparameter is properly added toBaseRetrieverwith a sensible default ofTrue. This enables retrieval of adjacent chunks to surface additional context, as described in the PR objectives.
43-56: LGTM!The
with_surrounding_chunksparameter is correctly propagated to the vector DB search call, maintaining consistency with the updatedasync_searchsignature invectordb.py.
59-60: LGTM!The typo fix from
SingleRetreivertoSingleRetrievercorrects the class name spelling and is reflected in the factory mapping at line 147.
86-102: LGTM!The
with_surrounding_chunksparameter is correctly propagated through the multi-query search flow, inheriting fromBaseRetrieverand passing it to the vector DB call.
129-142: LGTM!The
HyDeRetrievercorrectly propagates thewith_surrounding_chunksparameter through the hypothetical document search flow.
159-160: LGTM!Changing the validation from
retriever_type is Nonetoretriever_cls is Noneis more appropriate, as it checks the actual resolved class rather than the string identifier.openrag/components/indexer/vectordb/vectordb.py (8)
70-79: LGTM!The
with_surrounding_chunksparameter is properly added to both search method signatures with backward-compatible defaults. This extends the retrieval API to optionally include adjacent chunks.Also applies to: 82-91
118-135: LGTM!The addition of
[CHUNK_START],[CHUNK_END], and[CONTEXT]to the stop words list correctly prevents these formatting markers from influencing BM25 search results. These markers are introduced by the chunk formatting constants inchunker.py.
385-396: LGTM!The order metadata generation and attachment flow is well-implemented. Each chunk receives unique
section_id,prev_section_id, andnext_section_idfields that enable bidirectional linking for surrounding chunk retrieval. The use ofstrict=Truein the zip ensures all sequences have matching lengths.
427-455: LGTM!The multi-query search correctly propagates
with_surrounding_chunksto each individual search and deduplicates results by_id. This prevents duplicate chunks when multiple queries retrieve the same sections.
457-533: LGTM!The search implementation correctly:
- Performs vector/hybrid search with configured parameters
- Parses the search results into Document objects
- Optionally fetches and appends surrounding chunks when
with_surrounding_chunks=True- Returns the combined result set
The flow is clean and the conditional surrounding chunk retrieval is properly integrated.
555-602: LGTM!The
get_surrounding_chunksimplementation is well-designed:
- Efficiently collects prev/next section IDs from retrieved documents
- Queries all related sections in parallel using
asyncio.gather- Properly handles empty responses (line 586-587) to avoid
IndexError- Deduplicates by
_idto prevent returning chunks already in the result set- Constructs clean Document objects with appropriate metadata
1022-1037: LGTM!The
_gen_chunk_order_metadatafunction correctly generates unique section IDs by usingbase_ts + i, ensuring no duplicates even in rapid succession (addressing the previous review concern). The prev/next linking logic properly handles edge cases (first and last chunks).
1040-1055: LGTM!The
_parse_documents_from_search_resultshelper cleanly converts search results intoDocumentobjects, properly separatingpage_contentfrom metadata and excluding internal fields likevector.openrag/components/indexer/chunker/utils.py (8)
1-14: LGTM!The import additions and
PAGE_REpattern are correct. The regex properly matches page markers in the format[PAGE_N]where N is a digit, enabling page-aware chunking throughout the module.
17-31: LGTM!The
MDElementclass provides a clean, type-safe representation of markdown segments. The use ofLiteralfor the type field ensures only valid element types are used, and the optionalpage_numberaccommodates elements without page information.
34-50: LGTM!Both helper functions are correctly implemented:
span_insideproperly checks span containmentget_page_numbercorrectly interprets page markers, with the important semantic that content after[PAGE_N]belongs to page N+1
53-107: LGTM!The
split_md_elementsfunction is well-implemented:
- Correctly identifies page markers and assigns page numbers to elements
- Properly handles image/table precedence (skips tables inside image descriptions)
- Fills text segments between special elements
- Returns a clean list of
MDElementobjects
110-145: LGTM!The
get_chunk_page_numberfunction correctly determines the start and end pages for text chunks:
- Handles chunks with no page markers (entire chunk on previous page)
- Correctly interprets whether chunk starts/ends with a separator
- Returns a clear dict with start_page and end_page
148-188: LGTM!The
parse_markdown_tablefunction correctly parses markdown tables and groups rows by the Domain column (first column). The logic handles malformed rows gracefully and maintains group boundaries.
191-253: LGTM!The
chunk_tablefunction implements intelligent table chunking:
- Preserves the table header in each subtable
- Implements one-row overlap (line 223-224) to maintain context across chunks
- Uses
strict=Truein zip for safety (line 215)- Properly handles token size calculations to respect the chunk_size limit
The overlap strategy ensures users can understand continuity between consecutive table chunks.
256-280: LGTM!The
clean_markdown_table_spacingfunction properly normalizes table spacing by trimming cells and rebuilding rows with consistent spacing. This improves the consistency of table representations.openrag/components/indexer/chunker/chunker.py (7)
18-21: LGTM!The chunk format constants clearly separate contextualized and non-contextualized formats. The markers (
[CHUNK_START],[CHUNK_END],[CONTEXT]) are properly excluded from BM25 indexing via the stop words list invectordb.py.
24-66: LGTM!The
ChunkContextualizerclass is well-designed:
_generate_contextconstructs a rich prompt with document context (filename, first chunks, previous chunks)- Uses a semaphore to control concurrent VLM requests
- Gracefully handles errors by logging and returning empty context
- The broad exception catch (line 62) is appropriate here for resilience
68-105: LGTM!The
contextualize_chunksmethod efficiently processes chunks in parallel:
- Creates contextualization tasks for all chunks with appropriate first/previous chunk context
- Uses
tqdm.gatherfor progress feedback- Applies the
CHUNK_FORMATtemplate with generated context- Falls back to original chunks on error, ensuring robustness
108-134: LGTM!The
BaseChunkerinitialization is clean and efficient:
- Properly calculates chunk overlap from the overlap rate
- Uses the LLM's token counter for accurate length measurement
- Conditionally initializes the contextualizer only when needed
- The unused
kwargsparameter (line 117) allows subclass flexibility
135-192: LGTM!The helper methods are well-designed:
_apply_contextualizationapplies the appropriate format based on whether contextualization is enabled_prepare_md_elementsintelligently separates large tables/images while keeping small ones with textsplit_textprovides lazy initialization with a sensible fallback
194-254: LGTM!The
_get_chunksmethod implements the core chunking logic effectively:
- Separates MD elements into tables/images and text
- Chunks large tables with the specialized
chunk_tablefunction- Assigns page numbers to text chunks using
get_chunk_page_number- Sorts final chunks by page for consistent ordering
283-304: LGTM!The
RecursiveSplittercorrectly implements the recursive splitting strategy described in the PR objectives:
- First splits by newlines (paragraphs)
- Then by punctuation when paragraphs exceed chunk size
- The positive lookbehind regex
(?<=[\.\?\!])properly splits after sentence-ending punctuation
| async def split_document( | ||
| self, doc: Document, task_id: Optional[str] = None | ||
| ) -> list[Document]: | ||
| """Split document into chunks with optional contextualization.""" | ||
| metadata = doc.metadata | ||
| log = logger.bind( | ||
| file_id=metadata.get("file_id"), | ||
| partition=metadata.get("partition"), | ||
| task_id=task_id, | ||
| ) | ||
| log.info("Starting document chunking") | ||
| source = metadata["source"] | ||
|
|
||
| # Split the document into chunks of text, tables, and images | ||
| all_content = doc.page_content.strip() | ||
| splits = split_md_elements(all_content) | ||
| splits = combine_md_elements( | ||
| splits, llm=self.llm, chunk_max_size=self.chunk_size | ||
| ) | ||
|
|
||
| # Add overlap image and table chunks | ||
| splits = add_overlap( | ||
| chunks=splits, | ||
| target_chunk_types=["table", "image"], | ||
| add_before=True, | ||
| add_after=True, | ||
| chunk_overlap=self.chunk_overlap, | ||
| ) | ||
| detected_lang = detect_language(text=doc.page_content) | ||
|
|
||
| # only split text elements into chunks | ||
| chunks = [] | ||
| for chunk_type, content in splits: | ||
| if chunk_type == "text": | ||
| chunks.extend(self.split_text(content)) | ||
| else: | ||
| chunks.append(content) | ||
| # Process document through pipeline | ||
| chunks = self._get_chunks(doc.page_content.strip(), metadata, log=log) | ||
|
|
||
| # regrouping chunks based on token length | ||
| chunks = combine_chunks( | ||
| chunks=chunks, llm=self.llm, chunk_max_size=self.chunk_size | ||
| # Apply contextualization if enabled | ||
| log.info( | ||
| "Contextualizing chunks", apply_contextualization=self.contextual_retrieval | ||
| ) | ||
| chunks = await self._apply_contextualization(chunks, lang=detected_lang) | ||
|
|
||
| chunks_w_context = chunks # Default to original chunks if no contextualization | ||
| if self.contextual_retrieval: | ||
| log.info("Contextualizing chunks") | ||
| chunks_w_context = await self._contextualize_chunks(chunks, source=source) | ||
|
|
||
| filtered_chunks = [] | ||
| prev_page_num = 1 | ||
| for chunk, chunk_w_context in zip(chunks, chunks_w_context): | ||
| if not chunk.strip(): # skip empty chunks | ||
| continue | ||
|
|
||
| page_info = self._get_chunk_page_info( | ||
| chunk_str=chunk, previous_page=prev_page_num | ||
| ) | ||
| start_page = page_info["start_page"] | ||
| end_page = page_info["end_page"] | ||
| prev_page_num = end_page | ||
| filtered_chunks.append( | ||
| Document( | ||
| page_content=chunk_w_context, | ||
| metadata={**metadata, "page": start_page}, | ||
| ) | ||
| ) | ||
| log.info("Document chunking completed") | ||
| return filtered_chunks | ||
| return chunks |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for imports and the detect_language function usage in chunker.py
head -100 openrag/components/indexer/chunker/chunker.py | grep -E "import|from"Repository: linagora/openrag
Length of output: 608
🏁 Script executed:
#!/bin/bash
# Search for _apply_contextualization signature and detected_lang usage
rg -A 10 "async def _apply_contextualization" openrag/components/indexer/chunker/ --type=pyRepository: linagora/openrag
Length of output: 1079
🏁 Script executed:
#!/bin/bash
# Search for detect_language function definition and implementation
rg -B 2 -A 5 "def detect_language" openrag/ --type=pyRepository: linagora/openrag
Length of output: 448
🏁 Script executed:
#!/bin/bash
# Find lang_detector initialization and any validation/fallback logic
rg -B 10 "def detect_language" openrag/components/utils.py -A 5 | head -50Repository: linagora/openrag
Length of output: 507
🏁 Script executed:
#!/bin/bash
# Search for lang_detector initialization
rg "lang_detector" openrag/components/ --type=py -B 3 -A 3Repository: linagora/openrag
Length of output: 907
🏁 Script executed:
#!/bin/bash
# Check imports at the top of utils.py
head -50 openrag/components/utils.pyRepository: linagora/openrag
Length of output: 1593
🏁 Script executed:
#!/bin/bash
# Get the full implementation of _apply_contextualization
rg -A 50 "async def _apply_contextualization" openrag/components/indexer/chunker/chunker.py --type=pyRepository: linagora/openrag
Length of output: 2008
🏁 Script executed:
#!/bin/bash
# Find contextualizer initialization in chunker
rg "self.contextualizer" openrag/components/indexer/chunker/chunker.py -B 2 -A 2Repository: linagora/openrag
Length of output: 361
🏁 Script executed:
#!/bin/bash
# Search for contextualize_chunks implementation
rg -B 2 "def contextualize_chunks" openrag/ --type=py -A 20Repository: linagora/openrag
Length of output: 1938
🏁 Script executed:
#!/bin/bash
# Find _generate_context implementation
rg -B 2 "async def _generate_context" openrag/components/indexer/chunker/chunker.py -A 30Repository: linagora/openrag
Length of output: 1218
Add language validation to handle unsupported language codes gracefully.
The detect_language function can return any of ~176 language codes from fasttext-langdetect, but _apply_contextualization and related methods expect Literal["en", "fr"]. Unsupported language codes are passed directly to the LLM prompt without validation, causing unpredictable behavior. Add explicit validation or mapping to fallback to a supported language (e.g., "en") when an unsupported code is detected.
🤖 Prompt for AI Agents
In openrag/components/indexer/chunker/chunker.py around lines 256 to 280, the
detected_lang from detect_language may be any fasttext code but downstream
contextualization expects only supported codes (Literal["en","fr"]), so validate
and coerce the detected language before calling _apply_contextualization: check
detected_lang against a small allowed set (e.g., {"en","fr"}), map known
close/variant codes if desired (e.g., "en-US" -> "en", "fr-CA" -> "fr"), and
otherwise set a sensible fallback like "en"; log a warning when coercing/using
the fallback so it's visible for debugging. Ensure the validated/coerced
language is passed into _apply_contextualization instead of the raw detected
value.
2 participants
Improved Chunking Strategy
This update introduces a smarter, more structured approach to document chunking. The parsed markdown is split into distinct element types: text, image_description, and tables.
Text
Images
Tables
Retrieval
When retrieving documents, adjacent chunks can be included optionally and reranked. By this, we might find even more relevant chunks that were not retrieved.
Generation
Answer generation now enforces strict context-size control to prevent token overflow and ensure stable responses.
Summary by CodeRabbit
New Features
Improvements
Configuration
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.