LCORE-1333: Define max chunks to retrieve for RAG#1765
Conversation
|
Warning Review limit reached
More reviews will be available in 28 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
are-ces
force-pushed
the
lcore-1333
branch
4 times, most recently
from
6fbe0e9 to
a18cf4c
Compare
TamiTakamiya
left a comment
TamiTakamiya
left a comment
There was a problem hiding this comment.
LGTM. Thanks for providing the clear PR description.
| | `OKP_RAG_MAX_CHUNKS` | 5 | Max chunks retrieved from OKP (Inline RAG) | | ||
| | `BYOK_RAG_MAX_CHUNKS` | 10 | Max chunks retrieved from BYOK stores (Inline RAG) | | ||
| | `TOOL_RAG_MAX_CHUNKS` | 10 | Max chunks retrieved via Tool RAG (`file_search`) | | ||
| | `RAG_CONTENT_LIMIT` | 10 | Hard upper bound on the final merged inline RAG chunks (BYOK + OKP) delivered to the LLM | |
There was a problem hiding this comment.
The word "default" implies there's an override — but there isn't. May be replace Default with Value? The language is a bit misleading for someone reading the docs expecting a config knob.
There was a problem hiding this comment.
May be we should turn these into config knobs.
There was a problem hiding this comment.
The existing naming convention is:
BYOK_RAG_MAX_CHUNKS
OKP_RAG_MAX_CHUNKS
TOOL_RAG_MAX_CHUNKS
All follow the pattern
Something like INLINE_RAG_MAX_CHUNKS or MERGED_RAG_MAX_CHUNKS ?
There was a problem hiding this comment.
Yes I agree on the config, I would expose the knobs there and change the naming to have:
BYOK_RAG_MAX_CHUNKS
OKP_RAG_MAX_CHUNKS
TOOL_RAG_MAX_CHUNKS
INLINE_RAG_MAX_CHUNKS # from RAG_CONTENT_LIMIT
@TamiTakamiya what do you think about this?
PS: IMO the config needs restructuring because it's a bit of a mess now 😄
There was a problem hiding this comment.
The word "default" implies there's an override — but there isn't. May be replace Default with Value? The language is a bit misleading for someone reading the docs expecting a config knob.
I will fix it if we don't go with config knobs
Add test_responses_byok_integration.py with integration tests covering inline RAG, tool RAG, combined RAG, score multiplier, chunk capping, and RAG_CONTENT_LIMIT enforcement for the /responses endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add RAG_CONTENT_LIMIT constant (default: 10) that caps the final merged BYOK + OKP output from build_rag_context. Per-source constants remain as fetch hints for the reranking pool. Tool RAG is unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Description
Add
RAG_CONTENT_LIMITconstant (default: 10) toconstants.pythat caps the final merged BYOK + OKP inline RAG output frombuild_rag_context. Per-source constants (BYOK_RAG_MAX_CHUNKS,OKP_RAG_MAX_CHUNKS) remain as fetch hints for the reranking pool — users can increase them to feed more candidates to the cross-encoder reranker before the final cap is applied.Tool RAG keeps its own
TOOL_RAG_MAX_CHUNKSconstant independently so that users can set different max chunk limits for tool RAG and inline RAG separately.Also adds
test_responses_byok_integration.py— the/responsesendpoint had no BYOK RAG integration tests. The new file covers inline RAG, tool RAG, combined RAG, score multiplier, chunk capping, andRAG_CONTENT_LIMITenforcement, matching the existing test coverage for/queryand/streaming_query.Type of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run make verifypasses all lintersRAG_CONTENT_LIMITcaps inline RAG chunks when set below per-source constants (patched to 3, feeds 10 chunks, asserts cap at 3)RAG_CONTENT_LIMIT