feat(context): make per-section char limits configurable and proportional#4
Conversation
…onal
Previously, hardcoded constants in prompt.ts (primaryDoc=1200, primaryFile=4000,
relatedDoc=320, relatedFile=1200) overrode any budget configuration and were
unreachable for models with 200K+ context windows. Additionally, when the context
budget exhausted, related file content was dropped entirely instead of preserving
a snippet.
Changes:
- Add primaryDocLimit, primaryFileLimit, relatedDocLimit, relatedFileLimit to
RetrievalConfig (all optional, null = use proportional default)
- Add CODERAG_PRIMARY_DOC_LIMIT, CODERAG_PRIMARY_FILE_LIMIT,
CODERAG_RELATED_DOC_LIMIT, CODERAG_RELATED_FILE_LIMIT env var support
- Replace hardcoded SectionLimits constants with a deriveSectionLimits() function
that scales defaults proportionally with maxContextChars
- fitRelatedContexts() now keeps a 200-char snippet instead of dropping to empty
when budget runs out
- buildContextPackage and buildMultiHopContextPackage now return { context, limits }
so prompt.ts gets the actual configured limits
For 200K context models, set CODERAG_MAX_CONTEXT_CHARS=160000 and explicit overrides
to taste. For small-context models, the warning message now reflects the actual
configured limits rather than compile-time constants.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 22 minutes and 21 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR parameterizes prompt/truncation sizes via a new SectionLimits contract derived from retrieval config, returns computed limits from context builders, threads limits through message builders and service code, and changes exhausted related-content handling to retain short snippets with updated warnings. ChangesParameterized Context Limits and Snippet Retention
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic context budgeting by allowing per-section character limits to be derived from the total context budget or explicitly overridden via configuration. It also improves the handling of exhausted context budgets by retaining a small snippet of file content instead of dropping it entirely. Feedback focuses on code maintainability, specifically recommending the reuse of the MultiHopRetrievalResult type and the deriveSectionLimits function to avoid duplication, as well as extracting the hardcoded snippet length into a shared constant.
| ContextPackage, | ||
| GraphSnapshot, | ||
| IndexedNodeDocument, | ||
| MultiHopRetrievalResult, | ||
| RetrievedNodeContext, | ||
| RetrievalConfig | ||
| } from "../types.js"; | ||
| import type { RetrievedNodeContext } from "../types.js"; | ||
| import type { SectionLimits } from "./prompt.js"; | ||
| import { FileCache } from "../store/file-cache.js"; | ||
| import { createRetrievedNodeContext } from "../retrieval/page-index.js"; |
There was a problem hiding this comment.
The MultiHopRetrievalResult type was removed from the imports but is still logically applicable to the retrieval results used in this file. Additionally, deriveSectionLimits is now duplicated from context-builder.ts and should be imported instead.
ContextPackage,
GraphSnapshot,
IndexedNodeDocument,
RetrievedNodeContext,
RetrievalConfig,
MultiHopRetrievalResult
} from "../types.js";
import type { SectionLimits } from "./prompt.js";
import { deriveSectionLimits } from "./context-builder.js";
import { FileCache } from "../store/file-cache.js";
import { createRetrievedNodeContext } from "../retrieval/page-index.js";| const buildMultiHopGraphSummary = ( | ||
| subQuestions: string[], | ||
| retrievalResult: MultiHopRetrievalResult, | ||
| retrievalResult: { deduplicatedNodes: BlueprintNode[]; retrievalMetadata: Array<{ subQuestion: string; primaryNode?: BlueprintNode; relatedNodes: BlueprintNode[]; filesReferenced: string[] }> }, |
There was a problem hiding this comment.
Using a complex inline type for retrievalResult reduces readability and maintainability. Since MultiHopRetrievalResult is already defined in src/types.ts and matches this structure, it should be used here.
| retrievalResult: { deduplicatedNodes: BlueprintNode[]; retrievalMetadata: Array<{ subQuestion: string; primaryNode?: BlueprintNode; relatedNodes: BlueprintNode[]; filesReferenced: string[] }> }, | |
| retrievalResult: MultiHopRetrievalResult, |
| question: string, | ||
| subQuestions: string[], | ||
| retrievalResult: MultiHopRetrievalResult, | ||
| retrievalResult: { deduplicatedNodes: BlueprintNode[]; primaryNodes: Array<BlueprintNode | undefined>; expandedNodes: BlueprintNode[]; retrievalMetadata: Array<{ subQuestion: string; primaryNode?: BlueprintNode; relatedNodes: BlueprintNode[]; filesReferenced: string[] }> }, |
There was a problem hiding this comment.
Using a complex inline type for retrievalResult reduces readability and maintainability. Since MultiHopRetrievalResult is already defined in src/types.ts and matches this structure, it should be used here.
| retrievalResult: { deduplicatedNodes: BlueprintNode[]; primaryNodes: Array<BlueprintNode | undefined>; expandedNodes: BlueprintNode[]; retrievalMetadata: Array<{ subQuestion: string; primaryNode?: BlueprintNode; relatedNodes: BlueprintNode[]; filesReferenced: string[] }> }, | |
| retrievalResult: MultiHopRetrievalResult, |
| const deriveSectionLimits = (retrieval: RetrievalConfig): SectionLimits => { | ||
| const mcc = retrieval.maxContextChars; | ||
| return { | ||
| primaryDoc: retrieval.primaryDocLimit ?? Math.round((mcc / 16000) * 1200), | ||
| primaryFile: retrieval.primaryFileLimit ?? Math.round((mcc / 16000) * 4000), | ||
| relatedDoc: retrieval.relatedDocLimit ?? Math.round((mcc / 16000) * 320), | ||
| relatedFile: retrieval.relatedFileLimit ?? Math.round((mcc / 16000) * 1200) | ||
| }; | ||
| }; |
There was a problem hiding this comment.
The deriveSectionLimits function is duplicated from src/llm/context-builder.ts. To ensure consistency and ease of maintenance, this function should be exported from a common location (or just from context-builder.ts) and imported here.
| if (budget <= 0) { | ||
| warnings.push(`Dropped file content for ${context.filePath} because the context budget was exhausted.`); | ||
| // Never drop entirely — truncate to at least a snippet so the LLM retains context. | ||
| const snippetLength = Math.min(200, context.fullFileContent.length); |
There was a problem hiding this comment.
The magic number 200 for the snippet length is hardcoded in both context-builder.ts and multi-hop-context-builder.ts. This should be defined as a shared constant (e.g., MIN_SNIPPET_CHARS) to improve readability and make it easier to tune the snippet size globally.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45e1b99785
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (budget <= 0) { | ||
| warnings.push(`Dropped file content for ${context.filePath} because the context budget was exhausted.`); | ||
| // Never drop entirely — truncate to at least a snippet so the LLM retains context. | ||
| const snippetLength = Math.min(200, context.fullFileContent.length); | ||
| const snippet = context.fullFileContent.slice(0, snippetLength); | ||
| warnings.push( | ||
| snippetLength > 0 | ||
| ? `File content exhausted for ${context.filePath}; kept first ${snippetLength} chars as snippet.` | ||
| : `Dropped file content for ${context.filePath} because the context budget was exhausted.` | ||
| ); | ||
| fittedContexts.push({ | ||
| ...context, | ||
| fullFileContent: "" | ||
| fullFileContent: snippet | ||
| }); |
There was a problem hiding this comment.
When a primary file consumes the remaining context budget (or maxContextChars is very small), this branch now adds up to 200 characters for every related node anyway. For queries with several related files, the returned context and the prompt built from it can exceed the configured retrieval.maxContextChars, whereas the previous behavior dropped content once the budget was exhausted. If snippets are desired, they need to be accounted for in the remaining budget or limited by an explicit separate cap.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/context-builder.test.ts (1)
92-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd one direct assertion for the new
limitsreturn contract.These tests only destructure
context, so regressions inbuildContextPackage(...).limitswould not be caught even though limits are now part of retrieval behavior. Add at least one explicitlimitsassertion in this suite.As per coding guidelines,
New indexing, retrieval, transport, or MCP behavior must include direct test coverage.Also applies to: 151-162, 213-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/context-builder.test.ts` around lines 92 - 103, The test calls buildContextPackage(...) but only destructures context, so add an explicit assertion for the new limits return field to prevent regressions; locate the test invocation of buildContextPackage in this file (the call that currently destructures { context }) and update it to also capture limits (e.g., const { context, limits } = await buildContextPackage(...)) then add one assertion checking expected limits shape/values (for example assert.strictEqual or expect toMatchObject against the known limits like { topK: 4, rerankK: 2, maxContextChars: 18 } or whichever canonical values this test should enforce); apply the same pattern to the other two test blocks referenced (around lines 151-162 and 213-224) to ensure each suite directly covers the limits contract.
🧹 Nitpick comments (2)
src/llm/multi-hop-context-builder.ts (1)
75-83: 💤 Low valueConsider importing
deriveSectionLimitsfromcontext-builder.tsinstead of duplicating it.The function exists in both files with identical logic. Since
context-builder.tsalready exports it (lines 48–62), the multi-hop context builder should import it rather than maintain a separate copy. This eliminates duplication and reduces maintenance burden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llm/multi-hop-context-builder.ts` around lines 75 - 83, Remove the duplicated deriveSectionLimits implementation from multi-hop-context-builder and import the shared deriveSectionLimits exported by context-builder instead; update the top of multi-hop-context-builder.ts to import { deriveSectionLimits } (and any needed types like RetrievalConfig/SectionLimits if not already imported), delete the local function block, and ensure all references in buildMultiHopContext (or whichever functions call deriveSectionLimits) now use the imported symbol so there is a single source of truth.src/test/prompt.test.ts (1)
51-56: ⚡ Quick winUse deliberately small/distinct limits in one case to verify limit application behavior.
With near-default values, these tests mainly validate the new function signature, not that
buildMessagesactually applies caller-provided limits. Add one case with tiny limits and assert truncation/excerpt length.As per coding guidelines,
New indexing, retrieval, transport, or MCP behavior must include direct test coverage.Also applies to: 59-60, 80-81, 107-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/prompt.test.ts` around lines 51 - 56, Add a focused test case in src/test/prompt.test.ts that uses a deliberately tiny and distinct limits object (e.g., testLimits with very small primaryDoc/primaryFile/relatedDoc/relatedFile values) and passes it into buildMessages to verify that caller-provided limits are actually applied; assert that returned message parts (titles, excerpts, or content slices) are truncated to the expected lengths (check exact excerpt/truncation lengths) and add similar assertions for the other existing cases flagged (the ones around the other testLimits uses) so the test suite verifies truncation behavior rather than only the signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/llm/context-builder.ts`:
- Around line 52-55: The computed proportional defaults primaryDocDefault,
primaryFileDefault, relatedDocDefault and relatedFileDefault can become 0 for
small max-context values (mcc); change each calculation to clamp the result to
at least 1 (e.g., wrap the existing Math.round((mcc / 16000) * ...) expression
with Math.max(1, ...)) so no section limit becomes zero and truncation remains
valid.
---
Outside diff comments:
In `@src/test/context-builder.test.ts`:
- Around line 92-103: The test calls buildContextPackage(...) but only
destructures context, so add an explicit assertion for the new limits return
field to prevent regressions; locate the test invocation of buildContextPackage
in this file (the call that currently destructures { context }) and update it to
also capture limits (e.g., const { context, limits } = await
buildContextPackage(...)) then add one assertion checking expected limits
shape/values (for example assert.strictEqual or expect toMatchObject against the
known limits like { topK: 4, rerankK: 2, maxContextChars: 18 } or whichever
canonical values this test should enforce); apply the same pattern to the other
two test blocks referenced (around lines 151-162 and 213-224) to ensure each
suite directly covers the limits contract.
---
Nitpick comments:
In `@src/llm/multi-hop-context-builder.ts`:
- Around line 75-83: Remove the duplicated deriveSectionLimits implementation
from multi-hop-context-builder and import the shared deriveSectionLimits
exported by context-builder instead; update the top of
multi-hop-context-builder.ts to import { deriveSectionLimits } (and any needed
types like RetrievalConfig/SectionLimits if not already imported), delete the
local function block, and ensure all references in buildMultiHopContext (or
whichever functions call deriveSectionLimits) now use the imported symbol so
there is a single source of truth.
In `@src/test/prompt.test.ts`:
- Around line 51-56: Add a focused test case in src/test/prompt.test.ts that
uses a deliberately tiny and distinct limits object (e.g., testLimits with very
small primaryDoc/primaryFile/relatedDoc/relatedFile values) and passes it into
buildMessages to verify that caller-provided limits are actually applied; assert
that returned message parts (titles, excerpts, or content slices) are truncated
to the expected lengths (check exact excerpt/truncation lengths) and add similar
assertions for the other existing cases flagged (the ones around the other
testLimits uses) so the test suite verifies truncation behavior rather than only
the signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1a3d15e-31df-4dc6-b9cf-08d1c8fdbe57
📒 Files selected for processing (8)
src/llm/context-builder.tssrc/llm/multi-hop-context-builder.tssrc/llm/prompt.tssrc/service/coderag.tssrc/service/config.tssrc/test/context-builder.test.tssrc/test/prompt.test.tssrc/types.ts
There was a problem hiding this comment.
Pull request overview
This PR makes context section character limits configurable (via config and env vars) and proportional to maxContextChars, and threads the derived limits through context-building into prompt formatting. It also changes budget-exhaustion behavior so related file content retains a short snippet instead of being dropped entirely.
Changes:
- Extend
RetrievalConfig+ config loading to support per-section char limit overrides (including env vars). - Replace hardcoded prompt section limits with passed-in
SectionLimits, returned by context builders. - Adjust context-budget fitting to keep a short related-file snippet when budget is exhausted, and update tests for new return shapes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds optional per-section retrieval limit fields to the config schema/type. |
| src/service/config.ts | Adds env var overrides for the new per-section limit fields. |
| src/llm/context-builder.ts | Derives SectionLimits from retrieval config and returns { context, limits }; keeps snippets on budget exhaustion. |
| src/llm/multi-hop-context-builder.ts | Returns { context, limits } for multi-hop; keeps snippets on budget exhaustion; includes local section-limit derivation. |
| src/llm/prompt.ts | Introduces SectionLimits and applies them during prompt formatting; updates message builders to require limits. |
| src/service/coderag.ts | Wires new { context, limits } return shape into prompt message construction. |
| src/test/context-builder.test.ts | Updates for new return shape and new snippet-on-exhaustion behavior. |
| src/test/prompt.test.ts | Updates prompt tests to pass explicit SectionLimits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** Per-section char limits. All optional — defaults scale with maxContextChars. */ | ||
| primaryDocLimit: z.number().int().positive().optional(), | ||
| primaryFileLimit: z.number().int().positive().optional(), | ||
| relatedDocLimit: z.number().int().positive().optional(), | ||
| relatedFileLimit: z.number().int().positive().optional() |
| retrieval: { | ||
| ...baseConfig.retrieval, | ||
| topK: parseNumber(process.env.CODERAG_TOP_K) ?? baseConfig.retrieval.topK, | ||
| rerankK: parseNumber(process.env.CODERAG_RERANK_K) ?? baseConfig.retrieval.rerankK, | ||
| maxContextChars: parseNumber(process.env.CODERAG_MAX_CONTEXT_CHARS) ?? baseConfig.retrieval.maxContextChars | ||
| maxContextChars: parseNumber(process.env.CODERAG_MAX_CONTEXT_CHARS) ?? baseConfig.retrieval.maxContextChars, | ||
| primaryDocLimit: parseNumber(process.env.CODERAG_PRIMARY_DOC_LIMIT) ?? baseConfig.retrieval.primaryDocLimit, | ||
| primaryFileLimit: parseNumber(process.env.CODERAG_PRIMARY_FILE_LIMIT) ?? baseConfig.retrieval.primaryFileLimit, | ||
| relatedDocLimit: parseNumber(process.env.CODERAG_RELATED_DOC_LIMIT) ?? baseConfig.retrieval.relatedDocLimit, | ||
| relatedFileLimit: parseNumber(process.env.CODERAG_RELATED_FILE_LIMIT) ?? baseConfig.retrieval.relatedFileLimit | ||
| }, |
| for (const context of relatedContexts) { | ||
| if (budget <= 0) { | ||
| warnings.push(`Dropped file content for ${context.filePath} because the context budget was exhausted.`); | ||
| // Never drop entirely — truncate to at least a snippet so the LLM retains context. | ||
| const snippetLength = Math.min(200, context.fullFileContent.length); | ||
| const snippet = context.fullFileContent.slice(0, snippetLength); |
| const fittedRelated: RetrievedNodeContext[] = []; | ||
| for (const ctx of relatedContexts) { | ||
| if (remainingBudget <= 0) { | ||
| fittedRelated.push({ ...ctx, fullFileContent: "" }); | ||
| const snippetLength = Math.min(200, ctx.fullFileContent.length); | ||
| const snippet = ctx.fullFileContent.slice(0, snippetLength); | ||
| warnings.push( |
| const buildMultiHopGraphSummary = ( | ||
| subQuestions: string[], | ||
| retrievalResult: MultiHopRetrievalResult, | ||
| retrievalResult: { deduplicatedNodes: BlueprintNode[]; retrievalMetadata: Array<{ subQuestion: string; primaryNode?: BlueprintNode; relatedNodes: BlueprintNode[]; filesReferenced: string[] }> }, | ||
| snapshot: GraphSnapshot |
| const deriveSectionLimits = (retrieval: RetrievalConfig): SectionLimits => { | ||
| const mcc = retrieval.maxContextChars; | ||
| return { | ||
| primaryDoc: retrieval.primaryDocLimit ?? Math.round((mcc / 16000) * 1200), | ||
| primaryFile: retrieval.primaryFileLimit ?? Math.round((mcc / 16000) * 4000), | ||
| relatedDoc: retrieval.relatedDocLimit ?? Math.round((mcc / 16000) * 320), | ||
| relatedFile: retrieval.relatedFileLimit ?? Math.round((mcc / 16000) * 1200) | ||
| }; | ||
| }; |
| const formatSubQuestionSection = ( | ||
| index: number, | ||
| question: string, | ||
| nodes: RetrievedNodeContext[] | ||
| nodes: RetrievedNodeContext[], | ||
| limits: SectionLimits | ||
| ): string => { |
- Add maxInputTokens to EmbeddingProvider interface - Gemini embedding-2: 8192 tokens (~32K chars) vs old hardcoded 2048 - ONNX all-MiniLM-L6-v2: 256 tokens (~1K chars) - local-hash: Infinity (no truncation) - Update default model from embedding-001 to embedding-2 - Tests updated to reflect new model name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Agent-Logs-Url: https://github.com/nehraa/CodeRag/sessions/4ce1ad34-4751-4702-877b-84f789486c08 Co-authored-by: nehraa <187715469+nehraa@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/gemini-embedder.test.ts (1)
1-229: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMissing test coverage for
maxInputTokensfield.The new
maxInputTokensfield introduced inGeminiEmbeddingProvider(line 26 of gemini-embedder.ts) is not tested. This field affects indexing behavior (used for truncation indocuments.ts), and the coding guideline requires direct test coverage for new indexing, retrieval, transport, or MCP behavior.Please add a test case verifying:
- The
maxInputTokensfield exists on the provider instance- It has the expected value (8192)
As per coding guidelines: "New indexing, retrieval, transport, or MCP behavior must include direct test coverage"
📋 Suggested test case
Add this test to verify the
maxInputTokensfield:+ it("exposes maxInputTokens for gemini-embedding-2", () => { + const provider = new GeminiEmbeddingProvider({ apiKey: "test-key" }); + expect(provider.maxInputTokens).toBe(8192); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/gemini-embedder.test.ts` around lines 1 - 229, Add a unit test in the existing describe("GeminiEmbeddingProvider") suite that constructs a GeminiEmbeddingProvider (e.g., new GeminiEmbeddingProvider() or with apiKey) and asserts the instance has a maxInputTokens property equal to 8192; reference the GeminiEmbeddingProvider constructor and the maxInputTokens field so the test explicitly checks provider.maxInputTokens === 8192 and fails if the value is missing or different.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/test/gemini-embedder.test.ts`:
- Around line 1-229: Add a unit test in the existing
describe("GeminiEmbeddingProvider") suite that constructs a
GeminiEmbeddingProvider (e.g., new GeminiEmbeddingProvider() or with apiKey) and
asserts the instance has a maxInputTokens property equal to 8192; reference the
GeminiEmbeddingProvider constructor and the maxInputTokens field so the test
explicitly checks provider.maxInputTokens === 8192 and fails if the value is
missing or different.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b7aa920-e8c1-4cf3-838b-607272cdf56b
📒 Files selected for processing (10)
.env.exampleREADME.mdsrc/cli/setup-wizard.tssrc/indexer/documents.tssrc/indexer/embedder.tssrc/indexer/gemini-embedder.tssrc/indexer/onnx-embedder.tssrc/service/config.tssrc/test/gemini-embedder.test.tssrc/types.ts
✅ Files skipped from review due to trivial changes (4)
- src/indexer/embedder.ts
- src/indexer/onnx-embedder.ts
- .env.example
- README.md
|
@copilot Verify each finding against current code. Fix only still-valid issues, skip the Outside diff comments:
|
Summary
primaryDocLimit,primaryFileLimit,relatedDocLimit,relatedFileLimittoRetrievalConfig(all optional — null = proportional default)CODERAG_PRIMARY_DOC_LIMIT,CODERAG_PRIMARY_FILE_LIMIT,CODERAG_RELATED_DOC_LIMIT,CODERAG_RELATED_FILE_LIMITconstsection limits inprompt.tswith aderiveSectionLimits()function that scales proportionally withmaxContextCharsfitRelatedContexts()now keeps a 200-char snippet instead of dropping to empty when budget runs outbuildContextPackage()andbuildMultiHopContextPackage()now return{ context, limits }so prompt formatting gets the actual configured limitsProblem
Previously
PRIMARY_DOC_CHAR_LIMIT = 1_200,PRIMARY_FILE_LIMIT = 4_000,RELATED_DOC_LIMIT = 320,RELATED_FILE_LIMIT = 1_200were hardcoded compile-time constants inprompt.ts. They:maxContextCharswas configuredAdditionally, when the context budget exhausted,
fitRelatedContexts()dropped file content to""entirely — losing all signal for the LLM.Solution
deriveSectionLimits()computes defaults proportional tomaxContextChars:primaryDoc→ 7.5% of maxContextChars (e.g., 1,200 @ 16K, 12,000 @ 160K)primaryFile→ 25% of maxContextCharsrelatedDoc→ 2% of maxContextCharsrelatedFile→ 7.5% of maxContextCharsExplicit overrides (
CODERAG_PRIMARY_DOC_LIMITetc.) always take precedence. For 200K context models, setCODERAG_MAX_CONTEXT_CHARS=160000and tune explicit overrides to taste.Test plan
context-builder.test.tsupdated for newbuildContextPackagereturn shapeprompt.test.tsupdated to passSectionLimitstobuildMessages🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Changes
Documentation
Tests