-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix embeddingsComputer: Cannot read properties of undefined (reading 'type') #1215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: rzhao271 <[email protected]>
Co-authored-by: rzhao271 <[email protected]>
Co-authored-by: rzhao271 <[email protected]>
embeddingsComputer
: Cannot read properties of undefined (reading 'type')
|
||
import { beforeEach, describe, expect, test, vi } from 'vitest'; | ||
import type { Progress, SettingsSearchProviderOptions, SettingsSearchResult } from 'vscode'; | ||
import { SettingsSearchResultKind } from 'vscode'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot the type and non-type vscode imports being separate causes a build error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 19e002e. Combined the type and value imports from 'vscode' into a single import statement using inline type annotations.
Co-authored-by: rzhao271 <[email protected]>
@copilot We are already using mocha and sinon. Make tests more like src\extension\conversation\vscode-node\test\conversationFeature.test.ts |
Co-authored-by: rzhao271 <[email protected]>
Converted to Mocha and Sinon in commit 74af4dc. The test now follows the same pattern as conversationFeature.test.ts using Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Plan
undefined
is being passed as an embedding value causingCannot read properties of undefined (reading 'type')
errorsettingsEditorSearchServiceImpl.ts
, whencomputeEmbeddings
returns an empty array, accessingembeddingResult.values[0]
returnsundefined
nClosestValues
Root Cause
The issue occurs in
src/extension/prompt/vscode-node/settingsEditorSearchServiceImpl.ts
at line 68 whereembeddingResult.values[0]
is accessed without checking if it exists. WhencomputeEmbeddings
returns anEmbeddings
object with an emptyvalues
array (which can happen with anonymous access or other failures),embeddingResult.values[0]
isundefined
. This undefined value is then passed through the chain:nClosestValues(undefined, 25)
rankEmbeddings(undefined, ...)
distance(undefined, otherEmbedding)
undefined.type
→ ErrorSolution
Added a comprehensive guard to check both:
embeddingResult.values.length === 0
(empty array)!embeddingResult.values[0]
(null/undefined first element)This ensures we handle both the empty array case and any potential edge cases where the first element could be null or undefined.
Changes Made
settingsEditorSearchServiceImpl.ts
): Added defensive validation to check for empty embeddings array or invalid first element before callingnClosestValues
enums.ts
,vscodeTypesShim.ts
,vscodeTypes.ts
): AddedSettingsSearchResultKind
enum to test shims to enable proper unit testing of the proposed APIsettingsEditorSearchServiceImpl.test.ts
): Created comprehensive tests using Mocha and Sinon (project standard) validating the fix handles empty embeddings gracefullyTesting
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.