refactor: use c.ID() at all modelcaps call sites; inject store for testability

Per reviewer request, replace all c.ModelConfig.Provider+"/"+c.ModelConfig.Model
expressions with c.ID() at the modelcaps lookup call sites in every provider.

Also wire store injection so regression tests can verify the full path:

anthropic/client.go:
  - Add modelsStore *modelsdev.Store field (nil = use real store)
  - Add convertDoc() method routing through store when set, using c.ID()
  - convertUserMultiContent now calls c.convertDoc()

anthropic/beta_converter.go:
  - convertBetaUserMultiContent calls c.convertDoc()

openai/client.go:
  - Add modelsStore field; convertMessages routes through ConvertMessagesFromStore
  - Responses API inline call uses c.ID()

oaistream/messages.go:
  - Add ConvertMessagesFromStore / ConvertMultiContentFromStore (store-injectable)
  - Internal convertMessagesWithStore / convertMultiContentWithStore helpers

dmr, gemini, bedrock:
  - Use c.ID() directly

Tests updated:
  - anthropic test constructs Client{modelsStore: store} and calls
    convertUserMultiContent — image present only if c.ID() is used
  - oaistream test calls ConvertMultiContentFromStore with qualified vs
    bare ID — image present only when provider prefix is included

Author: simonferquel-clanker

pkg/model/provider/anthropic/attachments_test.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/docker/docker-agent/pkg/attachment/modelcaps"
 	"github.com/docker/docker-agent/pkg/chat"
+	"github.com/docker/docker-agent/pkg/config/latest"
+	"github.com/docker/docker-agent/pkg/model/provider/base"
 	"github.com/docker/docker-agent/pkg/modelsdev"
 )
 
@@ -53,13 +55,15 @@ func TestConvertDocumentAnthropic_StrategyB64_PDF(t *testing.T) {
 }
 
 // TestConvertDocumentAnthropic_QualifiedIDRequired is the regression test for
-// the bug where callers passed a bare model name to convertDocument instead of
-// a "provider/model" qualified identifier, causing modelcaps to miss the model
-// and silently drop all image/PDF attachments.
+// the bug where convertUserMultiContent passed only c.ModelConfig.Model (bare
+// model name) to convertDocument instead of c.ModelConfig.Provider+"/"+c.ModelConfig.Model.
+// When the bare name was used, modelcaps.Load always missed the model and all
+// image/PDF attachments were silently dropped.
 //
-// It calls convertDocumentFromStore directly with an injected fake store so
-// the test exercises the full modelID → LoadFromStore → caps → blocks path
-// without hitting the network.
+// The test constructs a Client with Provider="anthropic" and Model="claude-sonnet-4-6",
+// injects a fake modelsdev.Store, and calls convertUserMultiContent directly.
+// The image block must be present in the output — which only happens if the
+// fully-qualified "anthropic/claude-sonnet-4-6" was used for the caps lookup.
 func TestConvertDocumentAnthropic_QualifiedIDRequired(t *testing.T) {
 	store := modelsdev.NewDatabaseStore(&modelsdev.Database{
 		Providers: map[string]modelsdev.Provider{
@@ -75,24 +79,31 @@ func TestConvertDocumentAnthropic_QualifiedIDRequired(t *testing.T) {
 		},
 	})
 
-	doc := chat.Document{
-		Name:     "photo.jpg",
-		MimeType: "image/jpeg",
-		Source:   chat.DocumentSource{InlineData: minJPEG},
+	c := &Client{
+		Config: base.Config{
+			ModelConfig: latest.ModelConfig{
+				Provider: "anthropic",
+				Model:    "claude-sonnet-4-6",
+			},
+		},
+		modelsStore: store,
 	}
 
-	// Bare model name (the original bug): LoadFromStore misses the model,
-	// capabilities fall back to text-only, image must be dropped.
-	blocksBare, err := convertDocumentFromStore(t.Context(), doc, "claude-sonnet-4-6", store)
-	require.NoError(t, err)
-	assert.Nil(t, blocksBare, "bare model name must not resolve caps: image should be dropped")
+	parts := []chat.MessagePart{
+		{
+			Type: chat.MessagePartTypeDocument,
+			Document: &chat.Document{
+				Name:     "photo.jpg",
+				MimeType: "image/jpeg",
+				Source:   chat.DocumentSource{InlineData: minJPEG},
+			},
+		},
+	}
 
-	// Qualified ID (the fix): LoadFromStore finds the model, vision caps
-	// are set, image must be preserved as a native image block.
-	blocksQualified, err := convertDocumentFromStore(t.Context(), doc, "anthropic/claude-sonnet-4-6", store)
+	blocks, err := c.convertUserMultiContent(t.Context(), parts)
 	require.NoError(t, err)
-	require.Len(t, blocksQualified, 1, "qualified ID must resolve caps: image should be present")
-	assert.NotNil(t, blocksQualified[0].OfImage, "expected native image block for qualified model ID")
+	require.Len(t, blocks, 1, "image must not be dropped when provider+model ID is used for caps lookup")
+	assert.NotNil(t, blocks[0].OfImage, "expected native image block")
 }
 
 func TestConvertDocumentAnthropic_StrategyTXT(t *testing.T) {

pkg/model/provider/anthropic/beta_converter.go

@@ -277,7 +277,7 @@ func (c *Client) convertBetaUserMultiContent(ctx context.Context, parts []chat.M
 
 		case chat.MessagePartTypeDocument:
 			if part.Document != nil {
-				stdBlocks, err := convertDocument(ctx, *part.Document, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+				stdBlocks, err := c.convertDoc(ctx, *part.Document)
 				if err != nil {
 					return nil, fmt.Errorf("failed to convert document attachment %q: %w", part.Document.Name, err)
 				}

pkg/model/provider/anthropic/client.go

@@ -23,6 +23,7 @@ import (
 	"github.com/docker/docker-agent/pkg/model/provider/base"
 	"github.com/docker/docker-agent/pkg/model/provider/options"
 	"github.com/docker/docker-agent/pkg/model/provider/providerutil"
+	"github.com/docker/docker-agent/pkg/modelsdev"
 	"github.com/docker/docker-agent/pkg/tools"
 )
 
@@ -33,6 +34,7 @@ type Client struct {
 
 	clientFn    func(context.Context) (anthropic.Client, error)
 	fileManager *FileManager
+	modelsStore *modelsdev.Store // nil in production (uses modelsdev.NewStore()); set in tests
 }
 
 // NewClient creates a new Anthropic client from the provided configuration
@@ -313,6 +315,15 @@ func (c *Client) CreateChatCompletionStream(
 	return ad, nil
 }
 
+// convertDoc converts a document attachment using the client's model ID and
+// the injected store (if set) or the real modelsdev store.
+func (c *Client) convertDoc(ctx context.Context, doc chat.Document) ([]anthropic.ContentBlockParamUnion, error) {
+	if c.modelsStore != nil {
+		return convertDocumentFromStore(ctx, doc, c.ID(), c.modelsStore)
+	}
+	return convertDocument(ctx, doc, c.ID())
+}
+
 func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) ([]anthropic.MessageParam, error) {
 	var anthropicMessages []anthropic.MessageParam
 	// Track whether the last appended assistant message included tool_use blocks
@@ -553,7 +564,7 @@ func (c *Client) convertUserMultiContent(ctx context.Context, parts []chat.Messa
 
 		case chat.MessagePartTypeDocument:
 			if part.Document != nil {
-				docBlocks, err := convertDocument(ctx, *part.Document, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+				docBlocks, err := c.convertDoc(ctx, *part.Document)
 				if err != nil {
 					return nil, fmt.Errorf("failed to convert document attachment %q: %w", part.Document.Name, err)
 				}

pkg/model/provider/bedrock/client.go

@@ -236,7 +236,7 @@ func (c *Client) buildConverseStreamInput(ctx context.Context, messages []chat.M
 	enableCaching := c.promptCachingEnabled()
 
 	// Convert and set messages (excluding system)
-	input.Messages, input.System = convertMessages(ctx, messages, c.ModelConfig.Provider+"/"+c.ModelConfig.Model, enableCaching)
+	input.Messages, input.System = convertMessages(ctx, messages, c.ID(), enableCaching)
 
 	// Compute thinking fields first — its presence drives the inference config.
 	additionalFields := c.buildAdditionalModelRequestFields()

pkg/model/provider/dmr/client.go

@@ -147,7 +147,7 @@ func NewClient(ctx context.Context, cfg *latest.ModelConfig, opts ...options.Opt
 // convertMessages converts chat messages to OpenAI format and merges consecutive
 // system/user messages, which is needed by some local models run by DMR.
 func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) []openai.ChatCompletionMessageParamUnion {
-	openaiMessages := oaistream.ConvertMessages(ctx, messages, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+	openaiMessages := oaistream.ConvertMessages(ctx, messages, c.ID())
 	return oaistream.MergeConsecutiveMessages(openaiMessages)
 }
 

pkg/model/provider/gemini/client.go

@@ -601,7 +601,7 @@ func (c *Client) CreateChatCompletionStream(
 		}
 	}
 
-	contents := convertMessagesToGemini(ctx, messages, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+	contents := convertMessagesToGemini(ctx, messages, c.ID())
 
 	// Debug: Log the messages we're sending
 	slog.DebugContext(ctx, "Gemini messages", "count", len(contents))

pkg/model/provider/oaistream/attachments_test.go

@@ -58,9 +58,8 @@ func TestConvertDocument_StrategyB64_ImageDropped(t *testing.T) {
 // where callers passed a bare model name instead of a "provider/model" ID,
 // causing modelcaps to miss the model and silently drop image/PDF attachments.
 //
-// It calls convertDocumentFromStore directly with an injected fake store so
-// the test exercises the full modelID → LoadFromStore → caps → parts path
-// without hitting the network.
+// It calls ConvertMultiContentFromStore with an injected fake store, exercising
+// the same path as the production client (which calls ConvertMessages with c.ID()).
 func TestConvertDocument_QualifiedIDRequired(t *testing.T) {
 	store := modelsdev.NewDatabaseStore(&modelsdev.Database{
 		Providers: map[string]modelsdev.Provider{
@@ -76,22 +75,21 @@ func TestConvertDocument_QualifiedIDRequired(t *testing.T) {
 		},
 	})
 
-	doc := chat.Document{
-		Name:     "photo.jpg",
-		MimeType: "image/jpeg",
-		Source:   chat.DocumentSource{InlineData: minJPEG},
-	}
+	msgParts := []chat.MessagePart{{
+		Type: chat.MessagePartTypeDocument,
+		Document: &chat.Document{
+			Name:     "photo.jpg",
+			MimeType: "image/jpeg",
+			Source:   chat.DocumentSource{InlineData: minJPEG},
+		},
+	}}
 
-	// Bare model name (the original bug): LoadFromStore misses the model,
-	// capabilities fall back to text-only, image must be dropped.
-	partsBare, err := convertDocumentFromStore(t.Context(), doc, "gpt-4o", store)
-	require.NoError(t, err)
-	assert.Nil(t, partsBare, "bare model name must not resolve caps: image should be dropped")
+	// Bare model name (the original bug): image must be dropped.
+	partsBare := ConvertMultiContentFromStore(t.Context(), msgParts, "gpt-4o", store)
+	assert.Empty(t, partsBare, "bare model name must not resolve caps: image should be dropped")
 
-	// Qualified ID (the fix): LoadFromStore finds the model, vision caps
-	// are set, image must be preserved as a data-URI image part.
-	partsQualified, err := convertDocumentFromStore(t.Context(), doc, "openai/gpt-4o", store)
-	require.NoError(t, err)
+	// Qualified ID (the fix, matching what c.ID() returns): image must be preserved.
+	partsQualified := ConvertMultiContentFromStore(t.Context(), msgParts, "openai/gpt-4o", store)
 	require.Len(t, partsQualified, 1, "qualified ID must resolve caps: image should be present")
 	assert.NotNil(t, partsQualified[0].OfImageURL, "expected image URL part for qualified model ID")
 }

pkg/model/provider/oaistream/messages.go

@@ -14,6 +14,7 @@ import (
 	"github.com/openai/openai-go/v3/packages/param"
 
 	"github.com/docker/docker-agent/pkg/chat"
+	"github.com/docker/docker-agent/pkg/modelsdev"
 )
 
 // JSONSchema is a helper type that implements json.Marshaler for map[string]any.
@@ -30,6 +31,16 @@ func (j JSONSchema) MarshalJSON() ([]byte, error) {
 // modelID is used for attachment capability lookups; pass an empty string to
 // skip capability checks (all documents are attempted).
 func ConvertMultiContent(ctx context.Context, multiContent []chat.MessagePart, modelID string) []openai.ChatCompletionContentPartUnionParam {
+	return convertMultiContentWithStore(ctx, multiContent, modelID, nil)
+}
+
+// ConvertMultiContentFromStore is the store-injectable variant of ConvertMultiContent,
+// used by tests to inject a fake in-memory modelsdev.Store.
+func ConvertMultiContentFromStore(ctx context.Context, multiContent []chat.MessagePart, modelID string, store *modelsdev.Store) []openai.ChatCompletionContentPartUnionParam {
+	return convertMultiContentWithStore(ctx, multiContent, modelID, store)
+}
+
+func convertMultiContentWithStore(ctx context.Context, multiContent []chat.MessagePart, modelID string, store *modelsdev.Store) []openai.ChatCompletionContentPartUnionParam {
 	parts := make([]openai.ChatCompletionContentPartUnionParam, 0, len(multiContent))
 	for _, part := range multiContent {
 		switch part.Type {
@@ -45,7 +56,13 @@ func ConvertMultiContent(ctx context.Context, multiContent []chat.MessagePart, m
 			}
 		case chat.MessagePartTypeDocument:
 			if part.Document != nil {
-				docParts, err := convertDocument(ctx, *part.Document, modelID)
+				var docParts []openai.ChatCompletionContentPartUnionParam
+				var err error
+				if store != nil {
+					docParts, err = convertDocumentFromStore(ctx, *part.Document, modelID, store)
+				} else {
+					docParts, err = convertDocument(ctx, *part.Document, modelID)
+				}
 				if err != nil {
 					slog.WarnContext(ctx, "failed to convert document attachment", "error", err, "doc", part.Document.Name)
 					continue
@@ -62,6 +79,16 @@ func ConvertMultiContent(ctx context.Context, multiContent []chat.MessagePart, m
 // modelID is forwarded to convertDocument for attachment capability lookups.
 // This is the base conversion without any provider-specific post-processing.
 func ConvertMessages(ctx context.Context, messages []chat.Message, modelID string) []openai.ChatCompletionMessageParamUnion {
+	return convertMessagesWithStore(ctx, messages, modelID, nil)
+}
+
+// ConvertMessagesFromStore is the store-injectable variant of ConvertMessages,
+// used by tests to inject a fake in-memory modelsdev.Store.
+func ConvertMessagesFromStore(ctx context.Context, messages []chat.Message, modelID string, store *modelsdev.Store) []openai.ChatCompletionMessageParamUnion {
+	return convertMessagesWithStore(ctx, messages, modelID, store)
+}
+
+func convertMessagesWithStore(ctx context.Context, messages []chat.Message, modelID string, store *modelsdev.Store) []openai.ChatCompletionMessageParamUnion {
 	openaiMessages := make([]openai.ChatCompletionMessageParamUnion, 0, len(messages))
 	for i := range messages {
 		msg := &messages[i]
@@ -94,7 +121,7 @@ func ConvertMessages(ctx context.Context, messages []chat.Message, modelID strin
 			if len(msg.MultiContent) == 0 {
 				openaiMessage = openai.UserMessage(msg.Content)
 			} else {
-				openaiMessage = openai.UserMessage(ConvertMultiContent(ctx, msg.MultiContent, modelID))
+				openaiMessage = openai.UserMessage(convertMultiContentWithStore(ctx, msg.MultiContent, modelID, store))
 			}
 
 		case chat.MessageRoleAssistant:

pkg/model/provider/openai/client.go

@@ -26,6 +26,7 @@ import (
 	"github.com/docker/docker-agent/pkg/model/provider/oaistream"
 	"github.com/docker/docker-agent/pkg/model/provider/options"
 	"github.com/docker/docker-agent/pkg/modelinfo"
+	"github.com/docker/docker-agent/pkg/modelsdev"
 	"github.com/docker/docker-agent/pkg/rag/prompts"
 	"github.com/docker/docker-agent/pkg/rag/types"
 	"github.com/docker/docker-agent/pkg/tools"
@@ -41,6 +42,8 @@ type Client struct {
 	// wsPool is initialized in NewClient when transport=websocket is configured.
 	// It maintains a persistent WebSocket connection across requests.
 	wsPool *wsPool
+
+	modelsStore *modelsdev.Store // nil in production (uses modelsdev.NewStore()); set in tests
 }
 
 // NewClient creates a new OpenAI client from the provided configuration
@@ -181,7 +184,10 @@ func (c *Client) Close() {
 // convertMessages converts chat.Message to openai.ChatCompletionMessageParamUnion
 // using the shared oaistream implementation.
 func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) []openai.ChatCompletionMessageParamUnion {
-	return oaistream.ConvertMessages(ctx, messages, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+	if c.modelsStore != nil {
+		return oaistream.ConvertMessagesFromStore(ctx, messages, c.ID(), c.modelsStore)
+	}
+	return oaistream.ConvertMessages(ctx, messages, c.ID())
 }
 
 // CreateChatCompletionStream creates a streaming chat completion request
@@ -613,7 +619,7 @@ func (c *Client) convertMessagesToResponseInput(ctx context.Context, messages []
 						}
 					case chat.MessagePartTypeDocument:
 						if part.Document != nil {
-							docParts, err := convertDocumentToResponseInput(ctx, *part.Document, c.ModelConfig.Provider+"/"+c.ModelConfig.Model)
+							docParts, err := convertDocumentToResponseInput(ctx, *part.Document, c.ID())
 							if err != nil {
 								slog.WarnContext(ctx, "failed to convert document attachment", "error", err, "doc", part.Document.Name)
 								continue