Skip to content

Commit 2a5f75a

Browse files
ggoranov-smarggoranov-smar
committed
Fix various issues that the original PR missed.
1 parent 013496e commit 2a5f75a

File tree

11 files changed

+273
-40
lines changed

11 files changed

+273
-40
lines changed

‎packages/types/src/codebase-index.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export const codebaseIndexConfigSchema = z.object({
2222
codebaseIndexEnabled: z.boolean().optional(),
2323
codebaseIndexQdrantUrl: z.string().optional(),
2424
codebaseIndexEmbedderProvider: z
25-
.enum(["openai", "ollama", "openai-compatible", "gemini", "mistral", "vercel-ai-gateway"])
25+
.enum(["openai", "ollama", "openai-compatible", "gemini", "mistral", "vercel-ai-gateway", "bedrock"])
2626
.optional(),
2727
codebaseIndexEmbedderBaseUrl: z.string().optional(),
2828
codebaseIndexEmbedderModelId: z.string().optional(),
@@ -36,6 +36,9 @@ export const codebaseIndexConfigSchema = z.object({
3636
// OpenAI Compatible specific fields
3737
codebaseIndexOpenAiCompatibleBaseUrl: z.string().optional(),
3838
codebaseIndexOpenAiCompatibleModelDimension: z.number().optional(),
39+
// Bedrock specific fields
40+
codebaseIndexBedrockRegion: z.string().optional(),
41+
codebaseIndexBedrockProfile: z.string().optional(),
3942
})
4043

4144
export type CodebaseIndexConfig = z.infer<typeof codebaseIndexConfigSchema>

‎src/core/webview/ClineProvider.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,6 +1946,8 @@ export class ClineProvider
19461946
codebaseIndexOpenAiCompatibleBaseUrl: codebaseIndexConfig?.codebaseIndexOpenAiCompatibleBaseUrl,
19471947
codebaseIndexSearchMaxResults: codebaseIndexConfig?.codebaseIndexSearchMaxResults,
19481948
codebaseIndexSearchMinScore: codebaseIndexConfig?.codebaseIndexSearchMinScore,
1949+
codebaseIndexBedrockRegion: codebaseIndexConfig?.codebaseIndexBedrockRegion,
1950+
codebaseIndexBedrockProfile: codebaseIndexConfig?.codebaseIndexBedrockProfile,
19491951
},
19501952
// Only set mdmCompliant if there's an actual MDM policy
19511953
// undefined means no MDM policy, true means compliant, false means non-compliant
@@ -2164,6 +2166,8 @@ export class ClineProvider
21642166
stateValues.codebaseIndexConfig?.codebaseIndexOpenAiCompatibleBaseUrl,
21652167
codebaseIndexSearchMaxResults: stateValues.codebaseIndexConfig?.codebaseIndexSearchMaxResults,
21662168
codebaseIndexSearchMinScore: stateValues.codebaseIndexConfig?.codebaseIndexSearchMinScore,
2169+
codebaseIndexBedrockRegion: stateValues.codebaseIndexConfig?.codebaseIndexBedrockRegion,
2170+
codebaseIndexBedrockProfile: stateValues.codebaseIndexConfig?.codebaseIndexBedrockProfile,
21672171
},
21682172
profileThresholds: stateValues.profileThresholds ?? {},
21692173
includeDiagnosticMessages: stateValues.includeDiagnosticMessages ?? true,

‎src/core/webview/webviewMessageHandler.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2439,6 +2439,11 @@ export const webviewMessageHandler = async (
24392439

24402440
const settings = message.codeIndexSettings
24412441

2442+
// DEBUG: Log received settings
2443+
provider.log(
2444+
`[DEBUG] Received settings to save: provider=${settings.codebaseIndexEmbedderProvider}, bedrockRegion=${settings.codebaseIndexBedrockRegion}, bedrockProfile=${settings.codebaseIndexBedrockProfile}`,
2445+
)
2446+
24422447
try {
24432448
// Check if embedder provider has changed
24442449
const currentConfig = getGlobalState("codebaseIndexConfig") || {}
@@ -2455,10 +2460,17 @@ export const webviewMessageHandler = async (
24552460
codebaseIndexEmbedderModelId: settings.codebaseIndexEmbedderModelId,
24562461
codebaseIndexEmbedderModelDimension: settings.codebaseIndexEmbedderModelDimension, // Generic dimension
24572462
codebaseIndexOpenAiCompatibleBaseUrl: settings.codebaseIndexOpenAiCompatibleBaseUrl,
2463+
codebaseIndexBedrockRegion: settings.codebaseIndexBedrockRegion,
2464+
codebaseIndexBedrockProfile: settings.codebaseIndexBedrockProfile,
24582465
codebaseIndexSearchMaxResults: settings.codebaseIndexSearchMaxResults,
24592466
codebaseIndexSearchMinScore: settings.codebaseIndexSearchMinScore,
24602467
}
24612468

2469+
// DEBUG: Log what we're saving to global state
2470+
provider.log(
2471+
`[DEBUG] Saving to global state: bedrockRegion=${globalStateConfig.codebaseIndexBedrockRegion}, bedrockProfile=${globalStateConfig.codebaseIndexBedrockProfile}`,
2472+
)
2473+
24622474
// Save global state first
24632475
await updateGlobalState("codebaseIndexConfig", globalStateConfig)
24642476

@@ -2494,6 +2506,11 @@ export const webviewMessageHandler = async (
24942506
)
24952507
}
24962508

2509+
// DEBUG: Log what we're sending back to webview
2510+
provider.log(
2511+
`[DEBUG] Sending success response to webview: bedrockRegion=${globalStateConfig.codebaseIndexBedrockRegion}, bedrockProfile=${globalStateConfig.codebaseIndexBedrockProfile}`,
2512+
)
2513+
24972514
// Send success response first - settings are saved regardless of validation
24982515
await provider.postMessageToWebview({
24992516
type: "codeIndexSettingsSaved",

‎src/services/code-index/__tests__/config-manager.spec.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe("CodeIndexConfigManager", () => {
9898

9999
const result = await configManager.loadConfiguration()
100100

101-
expect(result.currentConfig).toEqual({
101+
expect(result.currentConfig).toMatchObject({
102102
isConfigured: false,
103103
embedderProvider: "openai",
104104
modelId: undefined,
@@ -129,7 +129,7 @@ describe("CodeIndexConfigManager", () => {
129129

130130
const result = await configManager.loadConfiguration()
131131

132-
expect(result.currentConfig).toEqual({
132+
expect(result.currentConfig).toMatchObject({
133133
isConfigured: true,
134134
embedderProvider: "openai",
135135
modelId: "text-embedding-3-large",
@@ -162,7 +162,7 @@ describe("CodeIndexConfigManager", () => {
162162

163163
const result = await configManager.loadConfiguration()
164164

165-
expect(result.currentConfig).toEqual({
165+
expect(result.currentConfig).toMatchObject({
166166
isConfigured: true,
167167
embedderProvider: "openai-compatible",
168168
modelId: "text-embedding-3-large",
@@ -199,7 +199,7 @@ describe("CodeIndexConfigManager", () => {
199199

200200
const result = await configManager.loadConfiguration()
201201

202-
expect(result.currentConfig).toEqual({
202+
expect(result.currentConfig).toMatchObject({
203203
isConfigured: true,
204204
embedderProvider: "openai-compatible",
205205
modelId: "custom-model",
@@ -237,7 +237,7 @@ describe("CodeIndexConfigManager", () => {
237237

238238
const result = await configManager.loadConfiguration()
239239

240-
expect(result.currentConfig).toEqual({
240+
expect(result.currentConfig).toMatchObject({
241241
isConfigured: true,
242242
embedderProvider: "openai-compatible",
243243
modelId: "custom-model",
@@ -275,7 +275,7 @@ describe("CodeIndexConfigManager", () => {
275275

276276
const result = await configManager.loadConfiguration()
277277

278-
expect(result.currentConfig).toEqual({
278+
expect(result.currentConfig).toMatchObject({
279279
isConfigured: true,
280280
embedderProvider: "openai-compatible",
281281
modelId: "custom-model",
@@ -1286,7 +1286,7 @@ describe("CodeIndexConfigManager", () => {
12861286

12871287
it("should return correct configuration via getConfig", () => {
12881288
const config = configManager.getConfig()
1289-
expect(config).toEqual({
1289+
expect(config).toMatchObject({
12901290
isConfigured: true,
12911291
embedderProvider: "openai",
12921292
modelId: "text-embedding-3-large",

‎src/services/code-index/config-manager.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ export class CodeIndexConfigManager {
139139
this.geminiOptions = geminiApiKey ? { apiKey: geminiApiKey } : undefined
140140
this.mistralOptions = mistralApiKey ? { apiKey: mistralApiKey } : undefined
141141
this.vercelAiGatewayOptions = vercelAiGatewayApiKey ? { apiKey: vercelAiGatewayApiKey } : undefined
142-
this.bedrockOptions = bedrockRegion
143-
? { region: bedrockRegion, profile: bedrockProfile || undefined }
144-
: undefined
142+
// Set bedrockOptions only if both region and profile are provided
143+
this.bedrockOptions =
144+
bedrockRegion && bedrockProfile ? { region: bedrockRegion, profile: bedrockProfile } : undefined
145145
}
146146

147147
/**
@@ -252,9 +252,11 @@ export class CodeIndexConfigManager {
252252
const isConfigured = !!(apiKey && qdrantUrl)
253253
return isConfigured
254254
} else if (this.embedderProvider === "bedrock") {
255+
// Both region and profile are required for Bedrock
255256
const region = this.bedrockOptions?.region
257+
const profile = this.bedrockOptions?.profile
256258
const qdrantUrl = this.qdrantUrl
257-
const isConfigured = !!(region && qdrantUrl)
259+
const isConfigured = !!(region && profile && qdrantUrl)
258260
return isConfigured
259261
}
260262
return false // Should not happen if embedderProvider is always set correctly

‎src/services/code-index/embedders/__tests__/bedrock.spec.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,25 +76,29 @@ describe("BedrockEmbedder", () => {
7676
send: mockSend,
7777
}))
7878

79-
embedder = new BedrockEmbedder("us-east-1", "amazon.titan-embed-text-v2:0")
79+
embedder = new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0")
8080
})
8181

8282
afterEach(() => {
8383
vitest.clearAllMocks()
8484
})
8585

8686
describe("constructor", () => {
87-
it("should initialize with provided region and model", () => {
87+
it("should initialize with provided region, profile and model", () => {
8888
expect(embedder.embedderInfo.name).toBe("bedrock")
8989
})
9090

91-
it("should use default region if not provided", () => {
92-
const defaultEmbedder = new BedrockEmbedder()
93-
expect(defaultEmbedder).toBeDefined()
91+
it("should require both region and profile", () => {
92+
expect(() => new BedrockEmbedder("", "profile", "model")).toThrow(
93+
"Both region and profile are required for AWS Bedrock embedder",
94+
)
95+
expect(() => new BedrockEmbedder("us-east-1", "", "model")).toThrow(
96+
"Both region and profile are required for AWS Bedrock embedder",
97+
)
9498
})
9599

96-
it("should use profile if provided", () => {
97-
const profileEmbedder = new BedrockEmbedder("us-west-2", undefined, "dev-profile")
100+
it("should use profile for credentials", () => {
101+
const profileEmbedder = new BedrockEmbedder("us-west-2", "dev-profile")
98102
expect(profileEmbedder).toBeDefined()
99103
})
100104
})
@@ -169,7 +173,7 @@ describe("BedrockEmbedder", () => {
169173
})
170174

171175
it("should handle Cohere model format", async () => {
172-
const cohereEmbedder = new BedrockEmbedder("us-east-1", "cohere.embed-english-v3")
176+
const cohereEmbedder = new BedrockEmbedder("us-east-1", "test-profile", "cohere.embed-english-v3")
173177
const testTexts = ["Hello world"]
174178
const mockResponse = {
175179
body: new TextEncoder().encode(

‎src/services/code-index/embedders/bedrock.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,21 @@ export class BedrockEmbedder implements IEmbedder {
2222

2323
/**
2424
* Creates a new AWS Bedrock embedder
25-
* @param region AWS region for Bedrock service
25+
* @param region AWS region for Bedrock service (required)
26+
* @param profile AWS profile name for credentials (required)
2627
* @param modelId Optional model ID override
27-
* @param profile Optional AWS profile name for credentials
2828
*/
2929
constructor(
30-
private readonly region: string = "us-east-1",
30+
private readonly region: string,
31+
private readonly profile: string,
3132
modelId?: string,
32-
private readonly profile?: string,
3333
) {
34-
// Initialize the Bedrock client with appropriate credentials
35-
const credentials = this.profile ? fromIni({ profile: this.profile }) : fromEnv()
34+
if (!region || !profile) {
35+
throw new Error("Both region and profile are required for AWS Bedrock embedder")
36+
}
37+
38+
// Initialize the Bedrock client with credentials from the specified profile
39+
const credentials = fromIni({ profile: this.profile })
3640

3741
this.bedrockClient = new BedrockRuntimeClient({
3842
region: this.region,

‎src/services/code-index/service-factory.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,11 @@ export class CodeIndexServiceFactory {
8181
}
8282
return new VercelAiGatewayEmbedder(config.vercelAiGatewayOptions.apiKey, config.modelId)
8383
} else if (provider === "bedrock") {
84-
if (!config.bedrockOptions?.region) {
84+
// Both region and profile are required for Bedrock
85+
if (!config.bedrockOptions?.region || !config.bedrockOptions?.profile) {
8586
throw new Error(t("embeddings:serviceFactory.bedrockConfigMissing"))
8687
}
87-
return new BedrockEmbedder(config.bedrockOptions.region, config.modelId, config.bedrockOptions.profile)
88+
return new BedrockEmbedder(config.bedrockOptions.region, config.bedrockOptions.profile, config.modelId)
8889
}
8990

9091
throw new Error(

‎src/shared/WebviewMessage.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,13 @@ export interface WebviewMessage {
288288
| "gemini"
289289
| "mistral"
290290
| "vercel-ai-gateway"
291+
| "bedrock"
291292
codebaseIndexEmbedderBaseUrl?: string
292293
codebaseIndexEmbedderModelId: string
293294
codebaseIndexEmbedderModelDimension?: number // Generic dimension for all providers
294295
codebaseIndexOpenAiCompatibleBaseUrl?: string
296+
codebaseIndexBedrockRegion?: string
297+
codebaseIndexBedrockProfile?: string
295298
codebaseIndexSearchMaxResults?: number
296299
codebaseIndexSearchMinScore?: number
297300

0 commit comments

Comments
 (0)