fix: respect UI-configured dimension settings for code indexing #7283
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove hardcoded 1536 fallback in getStateToPostToWebview() - Remove hardcoded fallback in getState() - Allow undefined dimension to flow through to service layer - Service layer properly resolves dimension from model profiles or user config Fixes #5754
dosubot
bot
added
size:XS
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| codebaseIndexEmbedderBaseUrl: codebaseIndexConfig?.codebaseIndexEmbedderBaseUrl ?? "", | ||
| codebaseIndexEmbedderModelId: codebaseIndexConfig?.codebaseIndexEmbedderModelId ?? "", | ||
| codebaseIndexEmbedderModelDimension: codebaseIndexConfig?.codebaseIndexEmbedderModelDimension ?? 1536, | ||
| codebaseIndexEmbedderModelDimension: codebaseIndexConfig?.codebaseIndexEmbedderModelDimension, |
Contributor
Author
There was a problem hiding this comment.
Good catch removing this hardcoded fallback! This was indeed preventing the UI-configured dimension from being used. However, I notice the PR description mentions TWO locations (line 1870 and lines 2059-2060), but I only see one change here. Did I miss updating the second location in the getState() method around line 2060?
| codebaseIndexEmbedderBaseUrl: codebaseIndexConfig?.codebaseIndexEmbedderBaseUrl ?? "", | ||
| codebaseIndexEmbedderModelId: codebaseIndexConfig?.codebaseIndexEmbedderModelId ?? "", | ||
| codebaseIndexEmbedderModelDimension: codebaseIndexConfig?.codebaseIndexEmbedderModelDimension ?? 1536, | ||
| codebaseIndexEmbedderModelDimension: codebaseIndexConfig?.codebaseIndexEmbedderModelDimension, |
Contributor
Author
There was a problem hiding this comment.
Since we're now allowing undefined to flow through, we should verify that the service layer properly handles this case. Looking at config-manager.ts and service-factory.ts, it appears the dimension resolution hierarchy is:
- User-configured dimension from UI
- Model-specific dimension from embedding profiles
- Error if no dimension can be determined
Could we add a test to ensure this hierarchy works correctly?
hannesrudolph
added
the
Issue/PR - Triage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes issue #5754 where RooCode was ignoring the dimension settings configured in the UI when creating file index collections.
Problem
The issue was that hardcoded fallback values (1536) in
ClineProvider.tswere overriding undefined dimension values, preventing the proper dimension resolution logic in the service layer from working correctly.Solution
Removed the hardcoded fallbacks in two locations:
getStateToPostToWebview()methodgetState()methodNow the dimension value flows through as undefined when not explicitly set, allowing the service layer to properly resolve it using this hierarchy:
Testing
Related Issue
Fixes #5754
Notes for Reviewers
@jhstatewide - This fix ensures that when you configure a dimension in the UI settings, it will be properly used when creating the Qdrant collection. The service layer (
config-manager.tsandservice-factory.ts) already has the correct logic to handle dimension resolution - the issue was just that the hardcoded fallback was preventing undefined values from reaching that logic.If you still experience issues after this fix, please let me know with details about:
Important
Remove hardcoded dimension fallback in
ClineProvider.tsto respect UI-configured settings for code indexing.getStateToPostToWebview()andgetState()inClineProvider.ts.This description was created by
for 104dbdb. You can customize this summary. It will automatically update as commits are pushed.