Skip to content

Commit 33a948c

Browse files
committed
fix: thread ctx + validate Titan v2 dimensions in ai/embed
Addresses CodeRabbit review on the Bedrock embeddings PR: - Thread the request context through EmbeddingConnector.getProvider into NewAWSConfig instead of context.Background(), so cancellation/deadlines aren't dropped during AWS config resolution (Major). - Validate Titan v2 dimensions up front — only 256, 512, or 1024 are accepted, so reject other values with a clear error instead of a cryptic Bedrock runtime failure (Minor). - Tests: cover the ai/embed bedrock provider path via a stubbed AWSConfigFunc (region resolution + [bedrock] error wrapping) and reject invalid v2 dimensions. (v1 dimension-ignoring is already covered by TestBedrockEmbeddingProvider_V1IgnoresDimensions.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WnKgCRfMvFcZAkoVbr8k79
1 parent cdff8d2 commit 33a948c

4 files changed

Lines changed: 58 additions & 3 deletions

File tree

packages/engine/internal/connector/embed.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (c *EmbeddingConnector) Execute(ctx context.Context, params map[string]any)
4848
return nil, fmt.Errorf("ai/embed: %w", err)
4949
}
5050

51-
provider, err := c.getProvider(providerName, params)
51+
provider, err := c.getProvider(ctx, providerName, params)
5252
if err != nil {
5353
return nil, fmt.Errorf("ai/embed: %w", err)
5454
}
@@ -80,7 +80,7 @@ func (c *EmbeddingConnector) Execute(ctx context.Context, params map[string]any)
8080
}
8181

8282
// getProvider returns the EmbeddingProvider for the given provider name.
83-
func (c *EmbeddingConnector) getProvider(name string, params map[string]any) (EmbeddingProvider, error) {
83+
func (c *EmbeddingConnector) getProvider(ctx context.Context, name string, params map[string]any) (EmbeddingProvider, error) {
8484
switch name {
8585
case "openai":
8686
baseURL := "https://api.openai.com/v1"
@@ -102,7 +102,7 @@ func (c *EmbeddingConnector) getProvider(name string, params map[string]any) (Em
102102
if configFunc == nil {
103103
configFunc = NewAWSConfig
104104
}
105-
awsCfg, err := configFunc(context.Background(), cred, defaultRegion)
105+
awsCfg, err := configFunc(ctx, cred, defaultRegion)
106106
if err != nil {
107107
return nil, fmt.Errorf("[bedrock]: %w", err)
108108
}

packages/engine/internal/connector/embed_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ package connector
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"net/http"
78
"net/http/httptest"
9+
"strings"
810
"testing"
11+
12+
"github.com/aws/aws-sdk-go-v2/aws"
913
)
1014

1115
// embedTestServer returns an httptest server that emits `n` deterministic
@@ -144,6 +148,32 @@ func TestEmbeddingConnector_IncompleteResponseFailsFast(t *testing.T) {
144148
}
145149
}
146150

151+
func TestEmbeddingConnector_BedrockRegionAndErrorWrapping(t *testing.T) {
152+
var gotRegion string
153+
c := &EmbeddingConnector{
154+
DefaultRegion: "us-west-2",
155+
AWSConfigFunc: func(ctx context.Context, cred map[string]string, defaultRegion string) (aws.Config, error) {
156+
gotRegion = defaultRegion
157+
return aws.Config{}, errors.New("boom")
158+
},
159+
}
160+
_, err := c.Execute(context.Background(), map[string]any{
161+
"model": "amazon.titan-embed-text-v2:0",
162+
"input": "x",
163+
"provider": "bedrock",
164+
"region": "eu-central-1", // params region overrides DefaultRegion
165+
})
166+
if err == nil {
167+
t.Fatal("expected error from stubbed AWS config")
168+
}
169+
if !strings.Contains(err.Error(), "[bedrock]") {
170+
t.Errorf("error not wrapped with [bedrock]: %v", err)
171+
}
172+
if gotRegion != "eu-central-1" {
173+
t.Errorf("resolved region = %q, want eu-central-1 (params override)", gotRegion)
174+
}
175+
}
176+
147177
func TestEmbeddingConnector_UnknownProvider(t *testing.T) {
148178
c := &EmbeddingConnector{}
149179
_, err := c.Execute(context.Background(), map[string]any{

packages/engine/internal/connector/provider_bedrock.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,13 @@ func (p *BedrockEmbeddingProvider) Embeddings(ctx context.Context, req *Embeddin
265265
// other than inputText, so ignore it there rather than failing a request
266266
// that reuses a shared embedding config.
267267
supportsDimensions := strings.Contains(req.Model, "titan-embed-text-v2")
268+
if req.Dimensions > 0 && supportsDimensions {
269+
switch req.Dimensions {
270+
case 256, 512, 1024:
271+
default:
272+
return nil, fmt.Errorf("bedrock: %s supports dimensions 256, 512, or 1024, got %d", req.Model, req.Dimensions)
273+
}
274+
}
268275

269276
for i, text := range req.Inputs {
270277
body := titanEmbedRequest{InputText: text}

packages/engine/internal/connector/provider_bedrock_embed_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,24 @@ func TestBedrockEmbeddingProvider_V1IgnoresDimensions(t *testing.T) {
9494
}
9595
}
9696

97+
func TestBedrockEmbeddingProvider_V2InvalidDimensions(t *testing.T) {
98+
mock := &mockBedrockInvokeClient{
99+
InvokeFunc: func(ctx context.Context, input *bedrockruntime.InvokeModelInput, opts ...func(*bedrockruntime.Options)) (*bedrockruntime.InvokeModelOutput, error) {
100+
t.Fatal("InvokeModel should not be called for invalid dimensions")
101+
return nil, nil
102+
},
103+
}
104+
p := &BedrockEmbeddingProvider{Client: mock}
105+
_, err := p.Embeddings(context.Background(), &EmbeddingRequest{
106+
Model: "amazon.titan-embed-text-v2:0",
107+
Inputs: []string{"x"},
108+
Dimensions: 999, // only 256/512/1024 are valid
109+
})
110+
if err == nil {
111+
t.Error("expected error for unsupported Titan v2 dimensions")
112+
}
113+
}
114+
97115
func TestBedrockEmbeddingProvider_UnsupportedModel(t *testing.T) {
98116
p := &BedrockEmbeddingProvider{Client: &mockBedrockInvokeClient{}}
99117
_, err := p.Embeddings(context.Background(), &EmbeddingRequest{

0 commit comments

Comments
 (0)