From 0e69bd18d9d1fd0dfe43b4c93a6586c7690516c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Fri, 24 Jul 2026 17:26:05 +0200 Subject: [PATCH 1/3] feat(#3996): detect audio/video input modalities Extend ModelCapabilities, CapsWith, CapsOverride and catalogue parsing with audio/video input support. Unknown models remain conservative. Update provider and attachment callers for the expanded capability shape without adding audio/video conversion support or changing the config schema. Retain the existing cancellable catalogue-load timeout and conservative fallback; waiting for the shared store lock remains outside that timeout. Cover modality and MIME matching, overrides, and fallback behavior. --- pkg/attachment/decide_test.go | 6 +- pkg/model/provider/anthropic/attachments.go | 2 +- .../provider/anthropic/attachments_test.go | 6 +- pkg/model/provider/base/base.go | 4 + .../provider/bedrock/attachments_test.go | 6 +- pkg/model/provider/dmr/attachments_test.go | 4 +- pkg/model/provider/dmr/client.go | 4 +- pkg/model/provider/gemini/attachments_test.go | 4 +- .../provider/oaistream/attachments_test.go | 12 +- pkg/modelinfo/modelinfo.go | 51 ++++- pkg/modelinfo/modelinfo_test.go | 174 +++++++++++++++++- 11 files changed, 243 insertions(+), 30 deletions(-) diff --git a/pkg/attachment/decide_test.go b/pkg/attachment/decide_test.go index c59a99696c..d8a342da6b 100644 --- a/pkg/attachment/decide_test.go +++ b/pkg/attachment/decide_test.go @@ -11,15 +11,15 @@ import ( // testCaps is a small helper that builds a ModelCapabilities directly. func visionCaps() modelinfo.ModelCapabilities { - return modelinfo.CapsWith(true, true) + return modelinfo.CapsWith(true, true, false, false) } func textOnlyCaps() modelinfo.ModelCapabilities { - return modelinfo.CapsWith(false, false) + return modelinfo.CapsWith(false, false, false, false) } func imageNoPDFCaps() modelinfo.ModelCapabilities { - return modelinfo.CapsWith(true, false) + return modelinfo.CapsWith(true, false, false, false) } func TestDecide(t *testing.T) { diff --git a/pkg/model/provider/anthropic/attachments.go b/pkg/model/provider/anthropic/attachments.go index 4456f81c99..a795acf33f 100644 --- a/pkg/model/provider/anthropic/attachments.go +++ b/pkg/model/provider/anthropic/attachments.go @@ -28,7 +28,7 @@ func convertDocument(ctx context.Context, doc chat.Document, id modelsdev.ID, st // An explicit config override is authoritative; only fall back to the // "any Claude model supports image+PDF" heuristic when none was declared. if override == nil && !mc.Supports(doc.MimeType) && modelinfo.IsClaude(ctx, store, id) { - mc = modelinfo.CapsWith(true, true) + mc = modelinfo.CapsWith(true, true, false, false) } return convertDocumentWithCaps(ctx, doc, mc) } diff --git a/pkg/model/provider/anthropic/attachments_test.go b/pkg/model/provider/anthropic/attachments_test.go index 03bb83559c..2fe5224ce4 100644 --- a/pkg/model/provider/anthropic/attachments_test.go +++ b/pkg/model/provider/anthropic/attachments_test.go @@ -31,7 +31,7 @@ func TestConvertDocumentAnthropic_StrategyB64_Image(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - visionCaps := modelinfo.CapsWith(true, true) + visionCaps := modelinfo.CapsWith(true, true, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, visionCaps) require.NoError(t, err) require.Len(t, blocks, 1, "expected exactly one block") @@ -49,7 +49,7 @@ func TestConvertDocumentAnthropic_StrategyB64_PDF(t *testing.T) { Source: chat.DocumentSource{InlineData: minPDF}, } - pdfCaps := modelinfo.CapsWith(true, true) + pdfCaps := modelinfo.CapsWith(true, true, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, pdfCaps) require.NoError(t, err) require.Len(t, blocks, 1, "expected exactly one block") @@ -167,7 +167,7 @@ func TestConvertDocumentAnthropic_Drop_UnsupportedMIME(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - textOnlyCaps := modelinfo.CapsWith(false, false) + textOnlyCaps := modelinfo.CapsWith(false, false, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, textOnlyCaps) require.NoError(t, err) assert.Nil(t, blocks, "image should be dropped for text-only model") diff --git a/pkg/model/provider/base/base.go b/pkg/model/provider/base/base.go index 6e694d958b..c21f981881 100644 --- a/pkg/model/provider/base/base.go +++ b/pkg/model/provider/base/base.go @@ -66,6 +66,10 @@ func (c *Config) TrackUsageEnabled() bool { // pass the result to [modelinfo.ResolveCaps] so a user-declared override wins // over a models.dev lookup that would otherwise miss for custom/aliased // providers and degrade attachments to text-only (issue #2741). +// +// [latest.CapabilitiesConfig] does not yet expose audio/video override flags, +// so Audio/Video are always false here; they flow through purely from +// models.dev detection until a later config change adds them. func (c *Config) CapsOverride() *modelinfo.CapsOverride { caps := c.ModelConfig.Capabilities if caps == nil { diff --git a/pkg/model/provider/bedrock/attachments_test.go b/pkg/model/provider/bedrock/attachments_test.go index 4d2e9a414f..679f6b1866 100644 --- a/pkg/model/provider/bedrock/attachments_test.go +++ b/pkg/model/provider/bedrock/attachments_test.go @@ -28,7 +28,7 @@ func TestConvertDocumentBedrock_StrategyB64_Image(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - visionCaps := modelinfo.CapsWith(true, true) + visionCaps := modelinfo.CapsWith(true, true, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, visionCaps) require.NoError(t, err) require.Len(t, blocks, 1, "expected exactly one block") @@ -50,7 +50,7 @@ func TestConvertDocumentBedrock_StrategyB64_PDF(t *testing.T) { Source: chat.DocumentSource{InlineData: minPDF}, } - pdfCaps := modelinfo.CapsWith(true, true) + pdfCaps := modelinfo.CapsWith(true, true, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, pdfCaps) require.NoError(t, err) require.Len(t, blocks, 1, "expected exactly one block") @@ -69,7 +69,7 @@ func TestConvertDocumentBedrock_StrategyB64_ImageDropped(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - textOnlyCaps := modelinfo.CapsWith(false, false) + textOnlyCaps := modelinfo.CapsWith(false, false, false, false) blocks, err := convertDocumentWithCaps(t.Context(), doc, textOnlyCaps) require.NoError(t, err) assert.Nil(t, blocks, "image should be dropped for text-only model") diff --git a/pkg/model/provider/dmr/attachments_test.go b/pkg/model/provider/dmr/attachments_test.go index e08c2ab43a..263f56540d 100644 --- a/pkg/model/provider/dmr/attachments_test.go +++ b/pkg/model/provider/dmr/attachments_test.go @@ -80,7 +80,7 @@ func TestDMRConvertMessagesRespectsDeclaredCaps(t *testing.T) { t.Run("image forwarded when supports_images declared", func(t *testing.T) { t.Parallel() - c := &Client{attachmentCaps: modelinfo.CapsWith(true, false)} + c := &Client{attachmentCaps: modelinfo.CapsWith(true, false, false, false)} msgs := c.convertMessages(t.Context(), docMessage("photo.png", "image/png", minPNG)) assert.Equal(t, 1, countImageParts(msgs), "image must be forwarded when supports_images is declared") }) @@ -94,7 +94,7 @@ func TestDMRConvertMessagesRespectsDeclaredCaps(t *testing.T) { t.Run("pdf forwarded as file part when supports_pdf declared", func(t *testing.T) { t.Parallel() - c := &Client{attachmentCaps: modelinfo.CapsWith(false, true)} + c := &Client{attachmentCaps: modelinfo.CapsWith(false, true, false, false)} msgs := c.convertMessages(t.Context(), docMessage("spec.pdf", "application/pdf", []byte("%PDF-1.4"))) assert.Equal(t, 1, countFileParts(msgs), "pdf must be forwarded as a file part when supports_pdf is declared") }) diff --git a/pkg/model/provider/dmr/client.go b/pkg/model/provider/dmr/client.go index 77b9fbabcc..661bc6b495 100644 --- a/pkg/model/provider/dmr/client.go +++ b/pkg/model/provider/dmr/client.go @@ -174,7 +174,7 @@ func NewClient(ctx context.Context, cfg *latest.ModelConfig, opts ...options.Opt client: openai.NewClient(clientOptions...), httpClient: httpClient, engine: engine, - attachmentCaps: modelinfo.CapsWith(parsed.supportsImages, parsed.supportsPDF), + attachmentCaps: modelinfo.CapsWith(parsed.supportsImages, parsed.supportsPDF, false, false), }, nil } @@ -190,7 +190,7 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) [ // (issue #2741). caps := c.attachmentCaps if override := c.CapsOverride(); override != nil { - caps = modelinfo.CapsWith(override.Image, override.PDF) + caps = modelinfo.CapsWith(override.Image, override.PDF, override.Audio, override.Video) } openaiMessages := oaistream.ConvertMessagesWithCaps(ctx, messages, caps) return oaistream.MergeConsecutiveMessages(openaiMessages) diff --git a/pkg/model/provider/gemini/attachments_test.go b/pkg/model/provider/gemini/attachments_test.go index 7fe78b7d10..0f8a196c2c 100644 --- a/pkg/model/provider/gemini/attachments_test.go +++ b/pkg/model/provider/gemini/attachments_test.go @@ -24,7 +24,7 @@ func TestConvertDocumentGemini_StrategyB64_Image(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - visionCaps := modelinfo.CapsWith(true, true) + visionCaps := modelinfo.CapsWith(true, true, false, false) part, err := convertDocumentWithCaps(t.Context(), doc, visionCaps) require.NoError(t, err) require.NotNil(t, part, "expected a non-nil part for B64 image") @@ -44,7 +44,7 @@ func TestConvertDocumentGemini_StrategyB64_ImageDropped(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - textOnlyCaps := modelinfo.CapsWith(false, false) + textOnlyCaps := modelinfo.CapsWith(false, false, false, false) part, err := convertDocumentWithCaps(t.Context(), doc, textOnlyCaps) require.NoError(t, err) assert.Nil(t, part, "image should be dropped for text-only model") diff --git a/pkg/model/provider/oaistream/attachments_test.go b/pkg/model/provider/oaistream/attachments_test.go index 60575fd174..65f2945b65 100644 --- a/pkg/model/provider/oaistream/attachments_test.go +++ b/pkg/model/provider/oaistream/attachments_test.go @@ -27,7 +27,7 @@ func TestConvertDocument_StrategyB64_Image(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - visionCaps := modelinfo.CapsWith(true, true) + visionCaps := modelinfo.CapsWith(true, true, false, false) parts, err := convertDocumentWithCaps(t.Context(), doc, visionCaps) require.NoError(t, err) require.Len(t, parts, 1, "expected exactly one image part") @@ -52,7 +52,7 @@ func TestConvertDocument_StrategyB64_PDF(t *testing.T) { Source: chat.DocumentSource{InlineData: pdf}, } - pdfCaps := modelinfo.CapsWith(false, true) + pdfCaps := modelinfo.CapsWith(false, true, false, false) parts, err := convertDocumentWithCaps(t.Context(), doc, pdfCaps) require.NoError(t, err) require.Len(t, parts, 1, "expected exactly one file part") @@ -75,7 +75,7 @@ func TestConvertDocument_StrategyB64_PDFDropped(t *testing.T) { Source: chat.DocumentSource{InlineData: []byte("%PDF-1.4")}, } - parts, err := convertDocumentWithCaps(t.Context(), doc, modelinfo.CapsWith(true, false)) + parts, err := convertDocumentWithCaps(t.Context(), doc, modelinfo.CapsWith(true, false, false, false)) require.NoError(t, err) assert.Nil(t, parts, "pdf should be dropped when the model does not support PDF") } @@ -97,7 +97,7 @@ func TestConvertMessagesWithCaps(t *testing.T) { }}, }} - withVision := ConvertMessagesWithCaps(t.Context(), messages, modelinfo.CapsWith(true, false)) + withVision := ConvertMessagesWithCaps(t.Context(), messages, modelinfo.CapsWith(true, false, false, false)) require.Len(t, withVision, 1) require.NotNil(t, withVision[0].OfUser) require.Len(t, withVision[0].OfUser.Content.OfArrayOfContentParts, 1) @@ -122,7 +122,7 @@ func TestConvertMessagesWithCaps(t *testing.T) { }}, }} - withPDF := ConvertMessagesWithCaps(t.Context(), pdfMessages, modelinfo.CapsWith(false, true)) + withPDF := ConvertMessagesWithCaps(t.Context(), pdfMessages, modelinfo.CapsWith(false, true, false, false)) require.Len(t, withPDF, 1) require.NotNil(t, withPDF[0].OfUser) require.Len(t, withPDF[0].OfUser.Content.OfArrayOfContentParts, 1) @@ -144,7 +144,7 @@ func TestConvertDocument_StrategyB64_ImageDropped(t *testing.T) { Source: chat.DocumentSource{InlineData: minJPEG}, } - textOnlyCaps := modelinfo.CapsWith(false, false) + textOnlyCaps := modelinfo.CapsWith(false, false, false, false) parts, err := convertDocumentWithCaps(t.Context(), doc, textOnlyCaps) require.NoError(t, err) assert.Nil(t, parts, "image should be dropped for text-only model") diff --git a/pkg/modelinfo/modelinfo.go b/pkg/modelinfo/modelinfo.go index 412d6cf582..669df44332 100644 --- a/pkg/modelinfo/modelinfo.go +++ b/pkg/modelinfo/modelinfo.go @@ -539,17 +539,41 @@ func isOSeries(m string) bool { type ModelCapabilities struct { supportsImage bool supportsPDF bool + supportsAudio bool + supportsVideo bool +} + +// SupportsImage reports whether the model accepts image attachments. +func (mc ModelCapabilities) SupportsImage() bool { + return mc.supportsImage +} + +// SupportsPDF reports whether the model accepts application/pdf attachments. +func (mc ModelCapabilities) SupportsPDF() bool { + return mc.supportsPDF +} + +// SupportsAudio reports whether the model accepts audio attachments. +func (mc ModelCapabilities) SupportsAudio() bool { + return mc.supportsAudio +} + +// SupportsVideo reports whether the model accepts video attachments. +func (mc ModelCapabilities) SupportsVideo() bool { + return mc.supportsVideo } // Supports reports whether the model can accept an attachment with the given // MIME type. // -// Only three content families are recognised: +// Only five content families are recognised: // - image/* → requires the models.dev "image" input modality // - application/pdf → requires the models.dev "pdf" input modality +// - audio/* → requires the models.dev "audio" input modality +// - video/* → requires the models.dev "video" input modality // - text/* → always accepted (TXT envelope is universally safe) // -// Everything else (audio, video, Office binaries, …) returns false. +// Everything else (Office binaries, …) returns false. func (mc ModelCapabilities) Supports(mimeType string) bool { mt := strings.ToLower(mimeType) switch { @@ -557,6 +581,10 @@ func (mc ModelCapabilities) Supports(mimeType string) bool { return mc.supportsImage case mt == "application/pdf": return mc.supportsPDF + case strings.HasPrefix(mt, "audio/"): + return mc.supportsAudio + case strings.HasPrefix(mt, "video/"): + return mc.supportsVideo case strings.HasPrefix(mt, "text/"): return true default: @@ -612,6 +640,8 @@ func ContextLimit(ctx context.Context, store *modelsdev.Store, id modelsdev.ID, type CapsOverride struct { Image bool PDF bool + Audio bool + Video bool } // ResolveCaps returns the model's attachment capabilities, preferring an @@ -624,7 +654,7 @@ type CapsOverride struct { // versions); see [github.com/docker/docker-agent/pkg/config/latest.CapabilitiesConfig]. func ResolveCaps(ctx context.Context, store *modelsdev.Store, id modelsdev.ID, override *CapsOverride) ModelCapabilities { if override != nil { - return CapsWith(override.Image, override.PDF) + return CapsWith(override.Image, override.PDF, override.Audio, override.Video) } return LoadCaps(ctx, store, id) } @@ -687,6 +717,10 @@ func LoadCaps(ctx context.Context, store *modelsdev.Store, id modelsdev.ID) Mode mc.supportsImage = true case "pdf": mc.supportsPDF = true + case "audio": + mc.supportsAudio = true + case "video": + mc.supportsVideo = true } } return mc @@ -695,9 +729,18 @@ func LoadCaps(ctx context.Context, store *modelsdev.Store, id modelsdev.ID) Mode // CapsWith constructs a ModelCapabilities value directly from booleans. This is // intended for use in tests and provider implementations that need to create a // capabilities value without hitting the network. -func CapsWith(supportsImage, supportsPDF bool) ModelCapabilities { +func CapsWith(supportsImage, supportsPDF bool, additional ...bool) ModelCapabilities { + supportsAudio, supportsVideo := false, false + if len(additional) > 0 { + supportsAudio = additional[0] + } + if len(additional) > 1 { + supportsVideo = additional[1] + } return ModelCapabilities{ supportsImage: supportsImage, supportsPDF: supportsPDF, + supportsAudio: supportsAudio, + supportsVideo: supportsVideo, } } diff --git a/pkg/modelinfo/modelinfo_test.go b/pkg/modelinfo/modelinfo_test.go index bb1624e035..7854d7e280 100644 --- a/pkg/modelinfo/modelinfo_test.go +++ b/pkg/modelinfo/modelinfo_test.go @@ -852,18 +852,49 @@ func TestLoadCaps_OfficeDocsNotAllowed(t *testing.T) { func TestCapsWith(t *testing.T) { t.Parallel() - mc := CapsWith(true, false) + mc := CapsWith(true, false, false, false) assert.True(t, mc.Supports("image/jpeg")) assert.False(t, mc.Supports("application/pdf")) - mc2 := CapsWith(false, false) + mc2 := CapsWith(false, false, false, false) assert.False(t, mc2.Supports("image/png")) } -func TestSupports_AudioVideoRejected(t *testing.T) { +// TestCapsWith_AudioVideo verifies the audio/video booleans passed to +// CapsWith flow through to Supports and to the dedicated accessors, and that +// each modality is independent of the others. +func TestCapsWith_AudioVideo(t *testing.T) { t.Parallel() - mc := CapsWith(true, true) + audioOnly := CapsWith(false, false, true, false) + assert.True(t, audioOnly.Supports("audio/mp3")) + assert.True(t, audioOnly.SupportsAudio()) + assert.False(t, audioOnly.Supports("video/mp4")) + assert.False(t, audioOnly.SupportsVideo()) + assert.False(t, audioOnly.Supports("image/jpeg")) + assert.False(t, audioOnly.Supports("application/pdf")) + + videoOnly := CapsWith(false, false, false, true) + assert.True(t, videoOnly.Supports("video/webm")) + assert.True(t, videoOnly.SupportsVideo()) + assert.False(t, videoOnly.Supports("audio/wav")) + assert.False(t, videoOnly.SupportsAudio()) + + all := CapsWith(true, true, true, true) + assert.True(t, all.SupportsImage()) + assert.True(t, all.SupportsPDF()) + assert.True(t, all.SupportsAudio()) + assert.True(t, all.SupportsVideo()) + + none := CapsWith(false, false, false, false) + assert.False(t, none.SupportsAudio()) + assert.False(t, none.SupportsVideo()) +} + +func TestSupports_AudioVideoRejectedWhenUnsupported(t *testing.T) { + t.Parallel() + + mc := CapsWith(true, true, false, false) for _, mime := range []string{ "audio/mp3", @@ -880,3 +911,138 @@ func TestSupports_AudioVideoRejected(t *testing.T) { "%q must not be supported", mime) } } + +// TestLoadCaps_AudioVideoModel verifies that "audio" and "video" models.dev +// input modalities are parsed into SupportsAudio/SupportsVideo, alongside a +// regression check that image/pdf parsing (added earlier) is unaffected. +func TestLoadCaps_AudioVideoModel(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{Providers: map[string]modelsdev.Provider{ + "google": { + Models: map[string]modelsdev.Model{ + "gemini-2.5-pro": { + Name: "Gemini 2.5 Pro", + Modalities: modelsdev.Modalities{ + Input: []string{"text", "image", "audio", "video", "pdf"}, + Output: []string{"text"}, + }, + }, + }, + }, + }}) + + mc := LoadCaps(t.Context(), store, modelsdev.NewID("google", "gemini-2.5-pro")) + + assert.True(t, mc.Supports("image/jpeg"), "regression: image parsing must still work") + assert.True(t, mc.Supports("application/pdf"), "regression: pdf parsing must still work") + assert.True(t, mc.Supports("audio/mp3")) + assert.True(t, mc.Supports("video/mp4")) + assert.True(t, mc.SupportsAudio()) + assert.True(t, mc.SupportsVideo()) +} + +// TestLoadCaps_AudioOnlyModel verifies that a model declaring only "audio" +// (no "video") does not also report video support, and vice versa. +func TestLoadCaps_AudioOnlyModel(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{Providers: map[string]modelsdev.Provider{ + "openai": { + Models: map[string]modelsdev.Model{ + "gpt-4o-audio-preview": { + Name: "GPT-4o Audio", + Modalities: modelsdev.Modalities{ + Input: []string{"text", "audio"}, + Output: []string{"text", "audio"}, + }, + }, + }, + }, + }}) + + mc := LoadCaps(t.Context(), store, modelsdev.NewID("openai", "gpt-4o-audio-preview")) + + assert.True(t, mc.SupportsAudio()) + assert.False(t, mc.SupportsVideo()) + assert.False(t, mc.SupportsImage()) + assert.False(t, mc.SupportsPDF()) +} + +// TestLoadCaps_UnknownModelHasNoAudioVideo is the conservative-default +// regression for audio/video: an uncatalogued model must not infer either +// modality, matching the existing image/PDF posture (#2741). +func TestLoadCaps_UnknownModelHasNoAudioVideo(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{Providers: map[string]modelsdev.Provider{}}) + + mc := LoadCaps(t.Context(), store, modelsdev.NewID("unknown", "nonexistent-model")) + + assert.False(t, mc.SupportsAudio()) + assert.False(t, mc.SupportsVideo()) + assert.False(t, mc.SupportsImage()) + assert.False(t, mc.SupportsPDF()) + assert.True(t, mc.Supports("text/plain")) +} + +// TestLoadCaps_MissingModalitiesHasNoAudioVideo covers a catalogued model +// whose Modalities.Input is present but lists neither "audio" nor "video" +// (only text/image): both new fields must stay conservative (false). +func TestLoadCaps_MissingModalitiesHasNoAudioVideo(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{Providers: map[string]modelsdev.Provider{ + "openai": { + Models: map[string]modelsdev.Model{ + "gpt-4o": { + Name: "GPT-4o", + Modalities: modelsdev.Modalities{ + Input: []string{"text", "image"}, + Output: []string{"text"}, + }, + }, + }, + }, + }}) + + mc := LoadCaps(t.Context(), store, modelsdev.NewID("openai", "gpt-4o")) + + assert.True(t, mc.SupportsImage()) + assert.False(t, mc.SupportsAudio()) + assert.False(t, mc.SupportsVideo()) +} + +// TestResolveCaps_OverrideAudioVideo verifies that CapsOverride's Audio/Video +// fields flow through ResolveCaps and take precedence over models.dev, mirroring +// the existing image/pdf override-precedence coverage in resolve_caps_test.go. +func TestResolveCaps_OverrideAudioVideo(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{Providers: map[string]modelsdev.Provider{ + "google": {Models: map[string]modelsdev.Model{ + "gemini-2.5-pro": {Modalities: modelsdev.Modalities{Input: []string{"text", "image", "audio", "video"}}}, + }}, + }}) + + // Override declares audio/video false even though the catalogue says + // true: the override is authoritative and wins. + override := &CapsOverride{Image: true, Audio: false, Video: false} + mc := ResolveCaps(t.Context(), store, modelsdev.NewID("google", "gemini-2.5-pro"), override) + assert.True(t, mc.SupportsImage()) + assert.False(t, mc.SupportsAudio()) + assert.False(t, mc.SupportsVideo()) + + // An override can also grant audio/video to an uncatalogued provider. + override2 := &CapsOverride{Audio: true, Video: true} + mc2 := ResolveCaps(t.Context(), store, modelsdev.NewID("custom-proxy", "some-model"), override2) + assert.True(t, mc2.SupportsAudio()) + assert.True(t, mc2.SupportsVideo()) + assert.False(t, mc2.SupportsImage()) + + // A nil override falls back to the models.dev lookup, which is fully + // multimodal for this model. + mc3 := ResolveCaps(t.Context(), store, modelsdev.NewID("google", "gemini-2.5-pro"), nil) + assert.True(t, mc3.SupportsAudio()) + assert.True(t, mc3.SupportsVideo()) +} From 366b6e849f4f29e25d24e34793012c8cc22aae20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Fri, 24 Jul 2026 18:00:43 +0200 Subject: [PATCH 2/3] feat(#3996): add audio/video capability overrides Extend latest.CapabilitiesConfig with Audio and Video and wire all four input flags through base.Config.CapsOverride. This lets custom or uncatalogued models override incomplete models.dev input metadata. Update agent-schema.json, model-reference docs, capability-overrides.yaml, and schema/config/provider tests. Numbered config packages stay frozen. --- agent-schema.json | 10 ++- docs/configuration/models/index.md | 21 ++++- examples/capability-overrides.yaml | 16 +++- pkg/config/latest/capabilities_test.go | 52 ++++++++++- pkg/config/latest/types.go | 7 +- pkg/config/schema_test.go | 88 +++++++++++++++++++ pkg/model/provider/base/base.go | 7 +- pkg/model/provider/base/base_test.go | 25 ++++++ .../provider/capability_override_test.go | 18 ++++ 9 files changed, 231 insertions(+), 13 deletions(-) diff --git a/agent-schema.json b/agent-schema.json index e30adc2be0..171b145673 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -1756,7 +1756,7 @@ }, "capabilities": { "$ref": "#/definitions/CapabilitiesConfig", - "description": "Explicit attachment capability override for models the models.dev catalogue does not describe correctly (custom OpenAI-compatible providers, local models like Ollama, or dropped model versions). When set, the declared flags are authoritative and no models.dev lookup is performed; when omitted, capabilities are detected automatically. Without it, such models fall back to text-only and their image/PDF attachments are silently dropped." + "description": "Explicit attachment capability override for models the models.dev catalogue does not describe correctly (custom OpenAI-compatible providers, local models like Ollama, or dropped model versions). When set, the declared flags are authoritative and no models.dev lookup is performed; when omitted, capabilities are detected automatically. Without it, such models fall back to text-only and their image/PDF/audio/video attachments are silently dropped." }, "cost": { "$ref": "#/definitions/CostConfig", @@ -1776,6 +1776,14 @@ "pdf": { "type": "boolean", "description": "Whether the model accepts PDF (application/pdf) attachments" + }, + "audio": { + "type": "boolean", + "description": "Whether the model accepts audio attachments" + }, + "video": { + "type": "boolean", + "description": "Whether the model accepts video attachments" } }, "additionalProperties": false diff --git a/docs/configuration/models/index.md b/docs/configuration/models/index.md index c909258b50..48257bd4d9 100644 --- a/docs/configuration/models/index.md +++ b/docs/configuration/models/index.md @@ -38,6 +38,8 @@ models: capabilities: # Optional: override attachment capabilities image: boolean # Optional: whether the model accepts image attachments pdf: boolean # Optional: whether the model accepts PDF attachments + audio: boolean # Optional: whether the model accepts audio attachments + video: boolean # Optional: whether the model accepts video attachments cost: # Optional: explicit token pricing (USD per 1M tokens) input: float # Optional: price per 1M input tokens output: float # Optional: price per 1M output tokens @@ -83,9 +85,9 @@ models: For custom OpenAI-compatible providers, local models (Ollama, DMR), and any model the built-in catalogue does not describe, Docker Agent cannot -auto-detect whether the endpoint accepts image or PDF attachments. When the -model is absent from the catalogue, Docker Agent logs a diagnostic and falls -back to text-only, silently dropping attachments. +auto-detect whether the endpoint accepts image, PDF, audio, or video +attachments. When the model is absent from the catalogue, Docker Agent logs a +diagnostic and falls back to text-only, silently dropping attachments. Declare `capabilities` to make the model's attachment support authoritative and skip the catalogue lookup entirely: @@ -105,12 +107,23 @@ models: capabilities: image: true pdf: true + + proxy-multimodal: + provider: vision-proxy + model: gemini-2.5-pro + capabilities: + image: true + pdf: true + audio: true + video: true ``` | Field | Type | Description | -| ---------------------- | ------- | ------------------------------------------------- | +| ---------------------- | ------- | -------------------------------------------------- | | `capabilities.image` | boolean | Whether the model accepts image attachments | | `capabilities.pdf` | boolean | Whether the model accepts PDF attachments | +| `capabilities.audio` | boolean | Whether the model accepts audio attachments | +| `capabilities.video` | boolean | Whether the model accepts video attachments | The flags must match what the endpoint actually accepts. Claiming a modality that the endpoint does not support leads to a provider-side API error. When diff --git a/examples/capability-overrides.yaml b/examples/capability-overrides.yaml index 8ef2905497..4ce1b57390 100644 --- a/examples/capability-overrides.yaml +++ b/examples/capability-overrides.yaml @@ -1,8 +1,8 @@ # Demonstrates the `capabilities` override for models the models.dev catalogue # does not describe correctly. Custom OpenAI-compatible providers and local # models are not (always) in models.dev, so docker-agent cannot auto-detect -# whether they accept image/PDF attachments and conservatively falls back to -# text-only, silently dropping attachments (issue #2741). +# whether they accept image/PDF/audio/video attachments and conservatively +# falls back to text-only, silently dropping attachments (issue #2741). # # Declaring `capabilities` makes the model's attachment support authoritative # and skips the models.dev lookup entirely. The flags must match what the @@ -38,6 +38,18 @@ models: image: true pdf: true + # Custom gateway serving a model that also accepts audio and video + # attachments (e.g. a self-hosted Gemini-family model behind a proxy that + # models.dev has never seen). + proxy-multimodal: + provider: vision-proxy + model: gemini-2.5-pro + capabilities: + image: true + pdf: true + audio: true + video: true + agents: root: model: llava-local diff --git a/pkg/config/latest/capabilities_test.go b/pkg/config/latest/capabilities_test.go index 151539b88c..c7ca947113 100644 --- a/pkg/config/latest/capabilities_test.go +++ b/pkg/config/latest/capabilities_test.go @@ -38,6 +38,54 @@ capabilities: assert.False(t, rt.Capabilities.PDF) } +func TestModelConfigCapabilitiesAudioVideoYAMLRoundTrip(t *testing.T) { + t.Parallel() + + const in = `provider: vision-proxy +model: gemini-2.5-pro +capabilities: + image: true + pdf: true + audio: true + video: true +` + var f FlexibleModelConfig + require.NoError(t, yaml.Unmarshal([]byte(in), &f)) + + require.NotNil(t, f.Capabilities, "capabilities should be parsed") + assert.True(t, f.Capabilities.Audio) + assert.True(t, f.Capabilities.Video) + + out, err := yaml.Marshal(f) + require.NoError(t, err) + + var rt FlexibleModelConfig + require.NoError(t, yaml.Unmarshal(out, &rt)) + require.NotNil(t, rt.Capabilities, "capabilities should survive a marshal round-trip; got:\n%s", out) + assert.True(t, rt.Capabilities.Audio) + assert.True(t, rt.Capabilities.Video) +} + +// TestModelConfigCapabilitiesAudioVideoOmittedDefaultsFalse pins that, within +// a present capabilities block, omitted audio/video resolve to false rather +// than falling back to models.dev: any non-nil block is authoritative. +func TestModelConfigCapabilitiesAudioVideoOmittedDefaultsFalse(t *testing.T) { + t.Parallel() + + const in = `provider: ollama +model: llava +capabilities: + image: true + pdf: false +` + var f FlexibleModelConfig + require.NoError(t, yaml.Unmarshal([]byte(in), &f)) + + require.NotNil(t, f.Capabilities) + assert.False(t, f.Capabilities.Audio) + assert.False(t, f.Capabilities.Video) +} + func TestModelConfigShorthandOnlyWithoutCapabilities(t *testing.T) { t.Parallel() @@ -80,13 +128,15 @@ func TestModelConfigCloneCopiesCapabilities(t *testing.T) { orig := &ModelConfig{ Provider: "my-proxy", Model: "gpt-4o", - Capabilities: &CapabilitiesConfig{Image: true, PDF: true}, + Capabilities: &CapabilitiesConfig{Image: true, PDF: true, Audio: true, Video: true}, } clone := orig.Clone() require.NotNil(t, clone.Capabilities) assert.True(t, clone.Capabilities.Image) assert.True(t, clone.Capabilities.PDF) + assert.True(t, clone.Capabilities.Audio) + assert.True(t, clone.Capabilities.Video) // Mutating the clone must not affect the original (deep copy). clone.Capabilities.Image = false diff --git a/pkg/config/latest/types.go b/pkg/config/latest/types.go index c6e54d4427..6a6b2140b1 100644 --- a/pkg/config/latest/types.go +++ b/pkg/config/latest/types.go @@ -1272,7 +1272,8 @@ func (c *CostConfig) validate() error { // catalogue). It exists for models the catalogue does not describe correctly: // custom OpenAI-compatible providers, local models (e.g. Ollama), and model // versions that have been dropped from the catalogue. Without it, such models -// fall back to text-only and their image/PDF attachments are silently dropped. +// fall back to text-only and their image/PDF/audio/video attachments are +// silently dropped. // // When set, the declared flags are authoritative and no models.dev lookup is // performed. When nil (the default), capabilities are detected from models.dev. @@ -1283,6 +1284,10 @@ type CapabilitiesConfig struct { Image bool `json:"image,omitempty"` // PDF reports whether the model accepts PDF (application/pdf) attachments. PDF bool `json:"pdf,omitempty"` + // Audio reports whether the model accepts audio attachments. + Audio bool `json:"audio,omitempty"` + // Video reports whether the model accepts video attachments. + Video bool `json:"video,omitempty"` } // IsFirstAvailable reports whether this model is a first-available selector diff --git a/pkg/config/schema_test.go b/pkg/config/schema_test.go index 2d81318c4b..082199212c 100644 --- a/pkg/config/schema_test.go +++ b/pkg/config/schema_test.go @@ -2,6 +2,7 @@ package config import ( "encoding/json" + "fmt" "maps" "os" "path/filepath" @@ -101,6 +102,93 @@ agents: {} } } +// TestJsonSchemaRejectsMalformedCapabilities pins that the capabilities +// override fields are strictly boolean: a non-boolean value (e.g. a string) +// for image/pdf/audio/video must fail schema validation rather than silently +// coercing, since a malformed override would otherwise reach the provider +// and surface as a confusing runtime error instead of an early config error. +func TestJsonSchemaRejectsMalformedCapabilities(t *testing.T) { + t.Parallel() + + schemaBytes, err := os.ReadFile(schemaFile) + require.NoError(t, err) + + schema, err := gojsonschema.NewSchema(gojsonschema.NewBytesLoader(schemaBytes)) + require.NoError(t, err) + + const base = `version: "15" +models: + m: + provider: openai + model: gpt-4o + capabilities: +%s +agents: + root: + model: m + instruction: hi +` + + tests := []struct { + name string + field string + }{ + {name: "image not boolean", field: " image: \"yes\"\n"}, + {name: "pdf not boolean", field: " pdf: \"yes\"\n"}, + {name: "audio not boolean", field: " audio: \"yes\"\n"}, + {name: "video not boolean", field: " video: \"yes\"\n"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var rawJSON any + require.NoError(t, yaml.Unmarshal(fmt.Appendf(nil, base, tt.field), &rawJSON)) + + result, err := schema.Validate(gojsonschema.NewRawLoader(rawJSON)) + require.NoError(t, err) + assert.False(t, result.Valid(), "expected schema to reject a non-boolean %s", tt.name) + }) + } +} + +// TestJsonSchemaAcceptsAudioVideoCapabilities confirms the audio/video +// capability override fields validate against the schema, both individually +// and together with image/pdf, mirroring how the runtime resolves them. +func TestJsonSchemaAcceptsAudioVideoCapabilities(t *testing.T) { + t.Parallel() + + schemaBytes, err := os.ReadFile(schemaFile) + require.NoError(t, err) + + schema, err := gojsonschema.NewSchema(gojsonschema.NewBytesLoader(schemaBytes)) + require.NoError(t, err) + + const in = `version: "15" +models: + m: + provider: openai + model: gpt-4o + capabilities: + image: true + pdf: true + audio: true + video: true +agents: + root: + model: m + instruction: hi +` + + var rawJSON any + require.NoError(t, yaml.Unmarshal([]byte(in), &rawJSON)) + + result, err := schema.Validate(gojsonschema.NewRawLoader(rawJSON)) + require.NoError(t, err) + assert.True(t, result.Valid(), "expected schema to accept audio/video capabilities: %v", result.Errors()) +} + // TestSchemaMatchesGoTypes verifies that every JSON-tagged field in the Go // config structs has a corresponding property in agent-schema.json (and // vice-versa). This prevents the schema from silently drifting out of sync diff --git a/pkg/model/provider/base/base.go b/pkg/model/provider/base/base.go index c21f981881..0cf64d7398 100644 --- a/pkg/model/provider/base/base.go +++ b/pkg/model/provider/base/base.go @@ -67,15 +67,14 @@ func (c *Config) TrackUsageEnabled() bool { // over a models.dev lookup that would otherwise miss for custom/aliased // providers and degrade attachments to text-only (issue #2741). // -// [latest.CapabilitiesConfig] does not yet expose audio/video override flags, -// so Audio/Video are always false here; they flow through purely from -// models.dev detection until a later config change adds them. +// All four fields (Image, PDF, Audio, Video) are read from the config's +// capabilities block when one is present. func (c *Config) CapsOverride() *modelinfo.CapsOverride { caps := c.ModelConfig.Capabilities if caps == nil { return nil } - return &modelinfo.CapsOverride{Image: caps.Image, PDF: caps.PDF} + return &modelinfo.CapsOverride{Image: caps.Image, PDF: caps.PDF, Audio: caps.Audio, Video: caps.Video} } // EmbeddingResult contains the embedding and usage information diff --git a/pkg/model/provider/base/base_test.go b/pkg/model/provider/base/base_test.go index 0a36d55802..464123fc17 100644 --- a/pkg/model/provider/base/base_test.go +++ b/pkg/model/provider/base/base_test.go @@ -30,4 +30,29 @@ func TestConfigCapsOverride(t *testing.T) { require.NotNil(t, got) assert.Equal(t, &modelinfo.CapsOverride{Image: true, PDF: false}, got) }) + + t.Run("mirrors declared audio/video capabilities", func(t *testing.T) { + t.Parallel() + c := &Config{ModelConfig: latest.ModelConfig{ + Provider: "vision-proxy", + Model: "gemini-2.5-pro", + Capabilities: &latest.CapabilitiesConfig{Image: true, PDF: true, Audio: true, Video: true}, + }} + got := c.CapsOverride() + require.NotNil(t, got) + assert.Equal(t, &modelinfo.CapsOverride{Image: true, PDF: true, Audio: true, Video: true}, got) + }) + + t.Run("omitted audio/video default to false", func(t *testing.T) { + t.Parallel() + c := &Config{ModelConfig: latest.ModelConfig{ + Provider: "ollama", + Model: "llava", + Capabilities: &latest.CapabilitiesConfig{Image: true, PDF: false}, + }} + got := c.CapsOverride() + require.NotNil(t, got) + assert.False(t, got.Audio) + assert.False(t, got.Video) + }) } diff --git a/pkg/model/provider/capability_override_test.go b/pkg/model/provider/capability_override_test.go index a7f2c674b1..34fa73732d 100644 --- a/pkg/model/provider/capability_override_test.go +++ b/pkg/model/provider/capability_override_test.go @@ -55,4 +55,22 @@ func TestCapabilityOverride_SurvivesProviderConstruction(t *testing.T) { bc := p.BaseConfig() assert.Nil(t, bc.CapsOverride()) }) + + t.Run("audio/video override flows through to the built provider", func(t *testing.T) { + t.Parallel() + cfg := &latest.ModelConfig{ + Provider: "openai", + Model: "gpt-4o", + BaseURL: "https://llm.internal.example.com/v1", + Capabilities: &latest.CapabilitiesConfig{Image: true, PDF: true, Audio: true, Video: true}, + } + + p, err := fullTestRegistry().New(t.Context(), cfg, env) + require.NoError(t, err) + + bc := p.BaseConfig() + got := bc.CapsOverride() + require.NotNil(t, got, "audio/video override must survive provider construction") + assert.Equal(t, &modelinfo.CapsOverride{Image: true, PDF: true, Audio: true, Video: true}, got) + }) } From 0180c777f1ea14e663520a7de9bda65aac262936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Fri, 24 Jul 2026 19:36:33 +0200 Subject: [PATCH 3/3] feat(#3996): strip unsupported audio/video media parts Generalize strip_unsupported_modalities across image, audio, and video parts using the capabilities of the model selected for each attempt. Prepare history again for each fallback provider rather than reusing the primary model's filtered copy, so a capable fallback retains user media. Keep stored history unchanged, preserve text and document ordering, and register the builtin by default for agents that bypass the YAML loader. Expose ModelAudioCapable and ModelVideoCapable to transform hooks. Unknown catalogue entries remain conservative for audio/video; retain DMR provider flags and the existing Anthropic/Claude image/PDF fallback. Update the configuration reference and example, and test MIME mapping, runtime-only agents, per-tool models, and primary/fallback capability differences. --- docs/configuration/models/index.md | 18 +- examples/strip-unsupported-media.yaml | 62 +++++ pkg/hooks/types.go | 13 ++ pkg/modelinfo/modelinfo.go | 26 ++- pkg/modelinfo/resolve_caps_test.go | 33 +++ pkg/runtime/fallback.go | 11 +- pkg/runtime/harness.go | 4 +- pkg/runtime/loop.go | 11 +- pkg/runtime/runtime.go | 1 + pkg/runtime/runtime_test.go | 52 ++++- pkg/runtime/strip_modalities.go | 178 +++++++++------ pkg/runtime/transforms.go | 66 +++++- pkg/runtime/transforms_test.go | 316 ++++++++++++++++++-------- 13 files changed, 607 insertions(+), 184 deletions(-) create mode 100644 examples/strip-unsupported-media.yaml diff --git a/docs/configuration/models/index.md b/docs/configuration/models/index.md index 48257bd4d9..03986f7d91 100644 --- a/docs/configuration/models/index.md +++ b/docs/configuration/models/index.md @@ -130,7 +130,23 @@ that the endpoint does not support leads to a provider-side API error. When `capabilities` is omitted the behaviour is unchanged (catalogue lookup then conservative text-only fallback). -See [`examples/capability-overrides.yaml`](https://github.com/docker/docker-agent/blob/main/examples/capability-overrides.yaml) for a complete example. +### Unsupported media is stripped before the call + +Before each model call, Docker Agent removes image, audio, and video message +parts that the resolved capabilities of the active model do not cover, instead +of letting the provider fail the whole request. Adjacent text (and PDF) parts +are preserved in their original order, and each stripped part is reported in +the debug log (`--debug`) with its media kind and reason. + +The stripping decision uses the same capability resolution as attachment +routing: an explicit `capabilities` declaration is authoritative, so a model +declared with `audio: true` keeps its audio parts even when the catalogue says +otherwise. Models absent from the catalogue (without an override) resolve to +the conservative text-only default and have their media parts stripped. + +See [`examples/capability-overrides.yaml`](https://github.com/docker/docker-agent/blob/main/examples/capability-overrides.yaml) for a complete example, and +[`examples/strip-unsupported-media.yaml`](https://github.com/docker/docker-agent/blob/main/examples/strip-unsupported-media.yaml) for a fixture demonstrating the +stripping behaviour with and without an override. ## Custom Token Pricing diff --git a/examples/strip-unsupported-media.yaml b/examples/strip-unsupported-media.yaml new file mode 100644 index 0000000000..7cd1d0da38 --- /dev/null +++ b/examples/strip-unsupported-media.yaml @@ -0,0 +1,62 @@ +# Mixed-media fixture for the strip_unsupported_modalities transform: the +# runtime drops image/audio/video message parts before the provider call when +# the resolved capabilities of the active model don't cover that media kind, +# and an explicit `capabilities` override always wins over the models.dev +# catalogue. +# +# Manual validation (needs OPENAI_API_KEY in the environment): +# +# task build +# +# # 1. Text-only model: the attached image is stripped (grep the debug log +# # for "strip_unsupported_modalities: stripped media part"); the text +# # part still reaches the model. +# ./bin/docker-agent run --exec examples/strip-unsupported-media.yaml -a text-only \ +# --attach --debug --log-file /tmp/strip-media.log \ +# "What do you see? Reply with one sentence." +# +# # 2. Vision model: no strip entry in the log; the model describes the image. +# ./bin/docker-agent run --exec examples/strip-unsupported-media.yaml -a vision \ +# --attach --debug --log-file /tmp/strip-media.log \ +# "What do you see? Reply with one sentence." +# +# # 3. Override: same text-only model, but `capabilities.image: true` makes +# # the declaration authoritative — no strip entry appears and the image +# # reaches the provider. (gpt-3.5-turbo genuinely rejects images, so the +# # provider-side 400 error is the PROOF that the override was respected +# # and the runtime no longer silently protected the call.) +# ./bin/docker-agent run --exec examples/strip-unsupported-media.yaml -a override \ +# --attach --debug --log-file /tmp/strip-media.log \ +# "What do you see? Reply with one sentence." +# +# Cleanup: rm /tmp/strip-media.log +models: + text-only: + provider: openai + model: gpt-3.5-turbo # models.dev input modalities: text only + + vision: + provider: openai + model: gpt-4o # models.dev input modalities: text, image, pdf + + claims-vision: + provider: openai + model: gpt-3.5-turbo + capabilities: + image: true # override wins: the runtime must NOT strip images + +agents: + text-only: + model: text-only + description: Text-only model — attached images are stripped before the call + instruction: You are a helpful assistant. Describe any images the user attaches. + + vision: + model: vision + description: Vision model — attached images reach the provider untouched + instruction: You are a helpful assistant. Describe any images the user attaches. + + override: + model: claims-vision + description: Text-only model with an explicit image capability override + instruction: You are a helpful assistant. Describe any images the user attaches. diff --git a/pkg/hooks/types.go b/pkg/hooks/types.go index b415bbaa73..15626fa5a9 100644 --- a/pkg/hooks/types.go +++ b/pkg/hooks/types.go @@ -8,6 +8,7 @@ import ( "encoding/json" "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/modelinfo" ) // EventType identifies a hook event. @@ -252,6 +253,18 @@ type Input struct { // model-call-scoped. ModelID string `json:"model_id,omitempty"` + // ModelCapabilities is the resolved attachment-capability set for + // [Input.ModelID], with any explicit `capabilities:` config override + // already applied — the same resolution providers use for attachment + // routing (see modelinfo.ResolveCapsFromModel). Like ModelID it is + // populated by the loop, but only for in-process before_llm_call + // message transforms; nil for every other event and for dispatch + // paths with no capability information (e.g. coding-harness labels). + // Capability-gated transforms must consume it instead of re-querying + // models.dev, which would ignore config overrides. Excluded from the + // JSON payload: cross-process hooks never see it. + ModelCapabilities *modelinfo.ModelCapabilities `json:"-"` + // Iteration is the 1-based run-loop iteration counter for the // model call this dispatch is gating. Populated for // [EventBeforeLLMCall] (1 for the first call of the RunStream, 2 diff --git a/pkg/modelinfo/modelinfo.go b/pkg/modelinfo/modelinfo.go index 669df44332..2216535320 100644 --- a/pkg/modelinfo/modelinfo.go +++ b/pkg/modelinfo/modelinfo.go @@ -710,9 +710,15 @@ func LoadCaps(ctx context.Context, store *modelsdev.Store, id modelsdev.ID) Mode return ModelCapabilities{} } + return capsFromModalities(model.Modalities.Input) +} + +// capsFromModalities maps a models.dev input-modality list to the capability +// booleans it grants. Unknown modality names are ignored. +func capsFromModalities(input []string) ModelCapabilities { var mc ModelCapabilities - for _, input := range model.Modalities.Input { - switch strings.ToLower(input) { + for _, modality := range input { + switch strings.ToLower(modality) { case "image": mc.supportsImage = true case "pdf": @@ -726,6 +732,22 @@ func LoadCaps(ctx context.Context, store *modelsdev.Store, id modelsdev.ID) Mode return mc } +// ResolveCapsFromModel applies the same precedence contract as [ResolveCaps] +// — an explicit override wins, otherwise capabilities derive from the +// models.dev record — for callers that fetch models through their own store +// abstraction (e.g. the runtime's ModelStore interface) instead of a concrete +// [*modelsdev.Store]. A nil model yields the same conservative text-only +// default as a store miss in [LoadCaps]. +func ResolveCapsFromModel(model *modelsdev.Model, override *CapsOverride) ModelCapabilities { + if override != nil { + return CapsWith(override.Image, override.PDF, override.Audio, override.Video) + } + if model == nil { + return ModelCapabilities{} + } + return capsFromModalities(model.Modalities.Input) +} + // CapsWith constructs a ModelCapabilities value directly from booleans. This is // intended for use in tests and provider implementations that need to create a // capabilities value without hitting the network. diff --git a/pkg/modelinfo/resolve_caps_test.go b/pkg/modelinfo/resolve_caps_test.go index 099d312a39..eb769dec6f 100644 --- a/pkg/modelinfo/resolve_caps_test.go +++ b/pkg/modelinfo/resolve_caps_test.go @@ -121,3 +121,36 @@ func TestLoadCaps_MissDiagnosticDedup(t *testing.T) { assert.Contains(t, buf.String(), "not found in models.dev") assert.Contains(t, buf.String(), "capabilities", "diagnostic should point at the config override") } + +// TestResolveCapsFromModel pins the store-free resolution path used by the +// runtime's strip transform: same precedence contract as ResolveCaps, but +// operating on an already-fetched models.dev record. +func TestResolveCapsFromModel(t *testing.T) { + t.Parallel() + + multimodal := &modelsdev.Model{Modalities: modelsdev.Modalities{Input: []string{"text", "image", "audio", "video"}}} + + cases := []struct { + name string + model *modelsdev.Model + override *CapsOverride + image, pdf, audio, video bool + }{ + {name: "modalities drive caps", model: multimodal, image: true, audio: true, video: true}, + {name: "override wins over modalities", model: multimodal, override: &CapsOverride{Audio: true}, audio: true}, + {name: "override wins over nil model", model: nil, override: &CapsOverride{Image: true, PDF: true, Audio: true, Video: true}, image: true, pdf: true, audio: true, video: true}, + {name: "nil model is conservative text-only", model: nil}, + {name: "empty modalities are conservative text-only", model: &modelsdev.Model{}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + mc := ResolveCapsFromModel(tc.model, tc.override) + assert.Equal(t, tc.image, mc.SupportsImage()) + assert.Equal(t, tc.pdf, mc.SupportsPDF()) + assert.Equal(t, tc.audio, mc.SupportsAudio()) + assert.Equal(t, tc.video, mc.SupportsVideo()) + }) + } +} diff --git a/pkg/runtime/fallback.go b/pkg/runtime/fallback.go index 96c9690bce..fe25990b22 100644 --- a/pkg/runtime/fallback.go +++ b/pkg/runtime/fallback.go @@ -41,6 +41,10 @@ type modelWithFallback struct { // mutates the executor directly, so the executor itself must exist // before opts run; the field assignments after opts complete the wiring. type fallbackExecutor struct { + // prepareMessages applies runtime message transforms for the provider + // selected for each attempt. It is set by [NewLocalRuntime]. + prepareMessages func(context.Context, *session.Session, *agent.Agent, provider.Provider, []chat.Message) []chat.Message + // retryOnRateLimit enables retry-with-backoff for HTTP 429 (rate limit) // errors when no fallback models are configured. When false (default), // 429 errors are treated as non-retryable and immediately fail or skip @@ -259,6 +263,11 @@ func (e *fallbackExecutor) execute( maxAttempts := 1 + fallbackRetries for attempt := range maxAttempts { + attemptMessages := messages + if e.prepareMessages != nil { + attemptMessages = e.prepareMessages(ctx, sess, a, modelEntry.provider, messages) + } + // Check context before each attempt if ctx.Err() != nil { fbSpan.SetOutcome(genai.FallbackOutcomeContextCanceled) @@ -306,7 +315,7 @@ func (e *fallbackExecutor) execute( // the goroutine reading the response body. streamCtx, streamCancel := context.WithCancelCause(ctx) - stream, err := modelEntry.provider.CreateChatCompletionStream(streamCtx, messages, agentTools) + stream, err := modelEntry.provider.CreateChatCompletionStream(streamCtx, attemptMessages, agentTools) if err != nil { streamCancel(nil) lastErr = err diff --git a/pkg/runtime/harness.go b/pkg/runtime/harness.go index 049258d754..62aad7ed69 100644 --- a/pkg/runtime/harness.go +++ b/pkg/runtime/harness.go @@ -60,7 +60,9 @@ func (r *LocalRuntime) runHarnessAgent(ctx context.Context, sess *session.Sessio if rewritten != nil { messages = rewritten } - messages = r.applyBeforeLLMCallTransforms(ctx, sess, a, modelID, messages) + // Harness labels are not models.dev identities and carry no resolved + // capabilities; capability-gated transforms skip on nil. + messages = r.applyBeforeLLMCallTransforms(ctx, sess, a, modelID, nil, messages) prompt := strings.TrimSpace(harnessPrompt(messages)) if prompt == "" { msg := "cannot run external harness without a user prompt" diff --git a/pkg/runtime/loop.go b/pkg/runtime/loop.go index 50dbe00443..18fd34842e 100644 --- a/pkg/runtime/loop.go +++ b/pkg/runtime/loop.go @@ -812,15 +812,8 @@ func (r *LocalRuntime) runTurn( messages = rewritten } - // Apply registered before_llm_call message transforms (e.g. - // strip_unsupported_modalities for text-only models, plus any - // embedder-supplied redactor / scrubber registered via - // WithMessageTransform). Runs after the gate so a transform - // failure cannot waste the gate's allow verdict. modelID is - // passed explicitly so transforms see the actual model the - // loop chose (per-tool override + alloy-mode selection), - // not whatever a fresh agent.Model() call would re-randomize. - messages = r.applyBeforeLLMCallTransforms(ctx, sess, a, modelID.String(), messages) + // Runtime message transforms run inside fallback.execute so each attempt + // uses the capabilities of the provider that will receive it. // Try primary model with fallback chain if configured agentTools = r.toolDeferrals.MarkAt(sess.ID, lastToolCallID(messages), agentTools) diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 241314679c..b378643374 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -712,6 +712,7 @@ func NewLocalRuntime(ctx context.Context, agents *team.Team, opts ...Opt) (*Loca dmrModelLister: dmrmodels.ListModels, } r.bgAgents = agenttool.NewHandler(r) + r.fallback.prepareMessages = r.prepareMessagesForModel // stripUnsupportedModalitiesTransform captures the runtime closure to // resolve the agent from Input.AgentName, so it lives here rather diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index baf9f5119a..2c47701ecc 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -24,6 +24,7 @@ import ( "github.com/docker/docker-agent/pkg/hooks" "github.com/docker/docker-agent/pkg/model/provider/base" "github.com/docker/docker-agent/pkg/modelerrors" + "github.com/docker/docker-agent/pkg/modelinfo" "github.com/docker/docker-agent/pkg/modelsdev" "github.com/docker/docker-agent/pkg/permissions" "github.com/docker/docker-agent/pkg/session" @@ -2992,11 +2993,12 @@ func TestSessionDenyOverridesYoloMode(t *testing.T) { require.False(t, executed, "expected tool to NOT be executed in --yolo mode because session Deny wins") } -func TestStripImageContent(t *testing.T) { +func TestStripUnsupportedMediaContent(t *testing.T) { t.Parallel() tests := []struct { name string + caps modelinfo.ModelCapabilities // zero value = text-only messages []chat.Message want []chat.Message }{ @@ -3180,12 +3182,58 @@ func TestStripImageContent(t *testing.T) { {Role: chat.MessageRoleAssistant, Content: "got it"}, }, }, + { + name: "strips audio and video documents, preserves supported image", + caps: modelinfo.CapsWith(true, false, false, false), + messages: []chat.Message{ + { + Role: chat.MessageRoleUser, + MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: "listen and watch"}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.wav", MimeType: "audio/wav", Source: chat.DocumentSource{InlineData: []byte{0x52}}}}, + {Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.mp4", MimeType: "video/mp4", Source: chat.DocumentSource{InlineData: []byte{0x00}}}}, + }, + }, + }, + want: []chat.Message{ + { + Role: chat.MessageRoleUser, + MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: "listen and watch"}, + {Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, + }, + }, + }, + }, + { + name: "retains audio and video when supported", + caps: modelinfo.CapsWith(false, false, true, true), + messages: []chat.Message{ + { + Role: chat.MessageRoleUser, + MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.mp3", MimeType: "audio/mpeg", Source: chat.DocumentSource{InlineData: []byte{0x49}}}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.webm", MimeType: "video/webm", Source: chat.DocumentSource{InlineData: []byte{0x1a}}}}, + }, + }, + }, + want: []chat.Message{ + { + Role: chat.MessageRoleUser, + MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.mp3", MimeType: "audio/mpeg", Source: chat.DocumentSource{InlineData: []byte{0x49}}}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.webm", MimeType: "video/webm", Source: chat.DocumentSource{InlineData: []byte{0x1a}}}}, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := stripImageContent(tt.messages) + got := stripUnsupportedMediaContent(t.Context(), tt.messages, tt.caps) require.Equal(t, tt.want, got) }) } diff --git a/pkg/runtime/strip_modalities.go b/pkg/runtime/strip_modalities.go index 404100f8f0..5867119778 100644 --- a/pkg/runtime/strip_modalities.go +++ b/pkg/runtime/strip_modalities.go @@ -3,90 +3,74 @@ package runtime import ( "context" "log/slog" - "slices" + "strings" "github.com/docker/docker-agent/pkg/chat" "github.com/docker/docker-agent/pkg/hooks" - "github.com/docker/docker-agent/pkg/modelsdev" + "github.com/docker/docker-agent/pkg/modelinfo" ) // BuiltinStripUnsupportedModalities is the name of the runtime-shipped -// before_llm_call message transform that drops image content from the -// outgoing messages when the agent's current model doesn't list image -// in its input modalities. It's the runtime-shipped peer of -// [BuiltinCacheResponse] (a stop hook) — the constant exists mostly -// for log filtering and diagnostics. +// before_llm_call message transform that drops image, audio, and video +// content from the outgoing messages when the resolved capabilities of +// the agent's current model don't cover that media kind. It's the +// runtime-shipped peer of [BuiltinCacheResponse] (a stop hook) — the +// constant exists mostly for log filtering and diagnostics. // -// Sending images to a text-only model produces hard provider errors -// (HTTP 400 from OpenAI, "image input is not supported" from -// Anthropic text variants, etc.); promoting the strip into a -// registered transform replaces an inline branch in runStreamLoop and -// opens the door to a family of message-mutating transforms -// (redactors, scrubbers, ...). +// Sending unsupported media produces hard provider errors (HTTP 400 +// from OpenAI, "image input is not supported" from Anthropic text +// variants, etc.); promoting the strip into a registered transform +// replaced an inline branch in runStreamLoop and opened the door to a +// family of message-mutating transforms (redactors, scrubbers, ...). const BuiltinStripUnsupportedModalities = "strip_unsupported_modalities" -// modalityImage is the canonical models.dev modality name for image -// input. A constant instead of a literal so a typo trips a compile -// error and the contract with [modelsdev.Modalities.Input] is -// discoverable from the runtime side. -const modalityImage = "image" - // stripUnsupportedModalitiesTransform is the [MessageTransform] -// registered under [BuiltinStripUnsupportedModalities]. It looks up -// the model definition from [hooks.Input.ModelID] (populated by the -// runtime with the actual model the loop chose, including per-tool -// overrides and alloy-mode selection) and applies -// [stripImageContent] when image is missing from the model's input -// modalities. +// registered under [BuiltinStripUnsupportedModalities]. It consumes +// the already-resolved capability set from +// [hooks.Input.ModelCapabilities] — populated by the loop for the +// model it actually chose (per-tool override + alloy-mode selection), +// with any explicit `capabilities:` config override applied — and +// drops the image/audio/video parts that model does not support. +// +// The transform must NOT resolve capabilities itself: a models.dev +// lookup here would ignore explicit config overrides, so a model the +// user declared `capabilities.audio: true` for would have its audio +// stripped anyway. Unknown models resolve (upstream, via +// [modelinfo.ResolveCapsFromModel]) to the conservative text-only +// default and lose their media parts — matching the attachment +// pipeline, which would drop them at provider conversion regardless. // -// The transform is a no-op for every "we don't know enough to act" -// case (missing ModelID, models.dev miss, empty modalities, image -// already supported): erring on the side of "send the messages -// as-is" matches the previous inline behavior in runStreamLoop, -// where an unknown model also fell through. Each fall-through emits -// a Debug log so operators can tell strip_unsupported_modalities -// from a transform that's silently inactive. +// A nil capability set means the dispatching path had nothing to +// resolve against (e.g. a coding-harness label, or an embedder-built +// Input); the messages then pass through untouched, with a Debug log +// so operators can tell that apart from a silently inactive transform. func (r *LocalRuntime) stripUnsupportedModalitiesTransform( ctx context.Context, in *hooks.Input, msgs []chat.Message, ) ([]chat.Message, error) { - if in == nil || in.ModelID == "" { - slog.DebugContext(ctx, "strip_unsupported_modalities: skipping, no ModelID on input") + if in == nil || in.ModelCapabilities == nil { + slog.DebugContext(ctx, "strip_unsupported_modalities: skipping, no resolved capabilities on input") return msgs, nil } - id, err := modelsdev.ParseID(in.ModelID) - if err != nil { - slog.DebugContext(ctx, "strip_unsupported_modalities: skipping, invalid ModelID", - "model_id", in.ModelID, "error", err) + mc := *in.ModelCapabilities + if mc.SupportsImage() && mc.SupportsAudio() && mc.SupportsVideo() { return msgs, nil } - m, err := r.modelsStore.GetModel(ctx, id) - if err != nil || m == nil { - // Unknown model: keep the previous (inline) behavior of - // passing messages through untouched. The model call will - // surface any modality mismatch as a provider error. - slog.DebugContext(ctx, "strip_unsupported_modalities: skipping, model definition unavailable", - "model_id", in.ModelID, "error", err) - return msgs, nil - } - if len(m.Modalities.Input) == 0 || slices.Contains(m.Modalities.Input, modalityImage) { - return msgs, nil - } - return stripImageContent(msgs), nil + return stripUnsupportedMediaContent(ctx, msgs, mc), nil } -// stripImageContent returns a copy of messages with all image-related -// content removed. Text content is preserved; image parts in -// [chat.Message.MultiContent] are filtered out, and file attachments -// with image MIME types are dropped. +// stripUnsupportedMediaContent returns a copy of messages with the +// media parts (image/audio/video) the model does not support removed. +// Text parts, PDFs, and any other non-media content are preserved, +// and the relative order of the surviving parts is unchanged. // // Lives next to [stripUnsupportedModalitiesTransform] (rather than in -// streaming.go where it originated) so the builtin's storage, -// transform, and helper are co-located. Kept as an unexported helper -// because the only legitimate caller is the transform itself — direct -// use bypasses the modality check. -func stripImageContent(messages []chat.Message) []chat.Message { +// streaming.go where its image-only ancestor originated) so the +// builtin's registration, transform, and helper are co-located. Kept +// as an unexported helper because the only legitimate caller is the +// transform itself — direct use bypasses the capability resolution. +func stripUnsupportedMediaContent(ctx context.Context, messages []chat.Message, mc modelinfo.ModelCapabilities) []chat.Message { result := make([]chat.Message, len(messages)) for i, msg := range messages { result[i] = msg @@ -97,27 +81,19 @@ func stripImageContent(messages []chat.Message) []chat.Message { var filtered []chat.MessagePart for _, part := range msg.MultiContent { - switch part.Type { - case chat.MessagePartTypeImageURL: - // Drop image URL parts entirely. + if kind := partMediaKind(part); kind != "" && !supportsMediaKind(mc, kind) { + slog.DebugContext(ctx, "strip_unsupported_modalities: stripped media part", + "kind", kind, + "role", msg.Role, + "reason", "model does not support "+kind+" input") continue - case chat.MessagePartTypeFile: - // Drop file parts that are images. - if part.File != nil && chat.IsImageMimeType(part.File.MimeType) { - continue - } - case chat.MessagePartTypeDocument: - // Drop Document parts that carry image InlineData. - if part.Document != nil && chat.IsImageMimeType(part.Document.MimeType) { - continue - } } filtered = append(filtered, part) } if len(filtered) != len(msg.MultiContent) { result[i].MultiContent = filtered - slog.Debug("Stripped image content from message", + slog.DebugContext(ctx, "Stripped media content from message", "role", msg.Role, "original_parts", len(msg.MultiContent), "remaining_parts", len(filtered)) @@ -125,3 +101,57 @@ func stripImageContent(messages []chat.Message) []chat.Message { } return result } + +// partMediaKind classifies a message part into the media kind gated by +// model input modalities: "image", "audio", or "video". Everything +// else — text parts, PDFs, unknown binaries — returns "" and is never +// touched by this transform (PDF gating stays at provider conversion). +// Legacy ImageURL parts carry no MIME type and are images by +// construction. +func partMediaKind(part chat.MessagePart) string { + switch part.Type { + case chat.MessagePartTypeImageURL: + return "image" + case chat.MessagePartTypeFile: + if part.File != nil { + return mimeMediaKind(part.File.MimeType) + } + case chat.MessagePartTypeDocument: + if part.Document != nil { + return mimeMediaKind(part.Document.MimeType) + } + } + return "" +} + +// mimeMediaKind maps a MIME type to "image", "audio", or "video" by +// family prefix — the same classification +// [modelinfo.ModelCapabilities.Supports] uses — or "" for any other +// type. +func mimeMediaKind(mimeType string) string { + switch mt := strings.ToLower(mimeType); { + case strings.HasPrefix(mt, "image/"): + return "image" + case strings.HasPrefix(mt, "audio/"): + return "audio" + case strings.HasPrefix(mt, "video/"): + return "video" + default: + return "" + } +} + +// supportsMediaKind reports whether the resolved capability set covers +// a media kind produced by [partMediaKind]. +func supportsMediaKind(mc modelinfo.ModelCapabilities, kind string) bool { + switch kind { + case "image": + return mc.SupportsImage() + case "audio": + return mc.SupportsAudio() + case "video": + return mc.SupportsVideo() + default: + return true + } +} diff --git a/pkg/runtime/transforms.go b/pkg/runtime/transforms.go index 3930bf2834..c9798472cc 100644 --- a/pkg/runtime/transforms.go +++ b/pkg/runtime/transforms.go @@ -3,10 +3,15 @@ package runtime import ( "context" "log/slog" + "strconv" "github.com/docker/docker-agent/pkg/agent" "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/hooks" + "github.com/docker/docker-agent/pkg/model/provider" + "github.com/docker/docker-agent/pkg/modelinfo" + "github.com/docker/docker-agent/pkg/modelsdev" "github.com/docker/docker-agent/pkg/session" ) @@ -71,22 +76,73 @@ func WithMessageTransform(name string, fn MessageTransform) Opt { // transforms read it via [hooks.Input.ModelID]. Calling // agent.Model() from a transform would re-randomize the alloy pick // and miss the per-tool override. +// +// caps is the model's already-resolved attachment capability set +// (explicit `capabilities:` config override applied — see +// [modelinfo.ResolveCapsFromModel]); transforms read it via +// [hooks.Input.ModelCapabilities]. nil means the caller has no +// capability information (e.g. the coding-harness path) and +// capability-gated transforms must not act. +func (r *LocalRuntime) prepareMessagesForModel( + ctx context.Context, + sess *session.Session, + a *agent.Agent, + model provider.Provider, + msgs []chat.Message, +) []chat.Message { + modelID := model.ID() + catalogModel, err := r.modelsStore.GetModel(ctx, modelID) + if err != nil { + slog.DebugContext(ctx, "Failed to resolve model capabilities for message transforms", "model", modelID.String(), "error", err) + } + cfg := model.BaseConfig() + caps := modelinfo.ResolveCapsFromModel(catalogModel, cfg.CapsOverride()) + if catalogModel == nil && cfg.CapsOverride() == nil { + caps = providerFallbackCaps(ctx, cfg.ModelConfig, modelID) + } + return r.applyBeforeLLMCallTransforms(ctx, sess, a, modelID.String(), &caps, msgs) +} + +func providerFallbackCaps(ctx context.Context, cfg latest.ModelConfig, id modelsdev.ID) modelinfo.ModelCapabilities { + if cfg.Provider == "dmr" { + return modelinfo.CapsWith(providerOptBool(cfg.ProviderOpts, "supports_images"), providerOptBool(cfg.ProviderOpts, "supports_pdf"), false, false) + } + if cfg.Provider == "anthropic" || modelinfo.IsClaude(ctx, nil, id) { + return modelinfo.CapsWith(true, true, false, false) + } + return modelinfo.ModelCapabilities{} +} + +func providerOptBool(opts map[string]any, key string) bool { + switch value := opts[key].(type) { + case bool: + return value + case string: + parsed, err := strconv.ParseBool(value) + return err == nil && parsed + default: + return false + } +} + func (r *LocalRuntime) applyBeforeLLMCallTransforms( ctx context.Context, sess *session.Session, a *agent.Agent, modelID string, + caps *modelinfo.ModelCapabilities, msgs []chat.Message, ) []chat.Message { if len(r.transforms) == 0 { return msgs } in := &hooks.Input{ - SessionID: sess.ID, - AgentName: a.Name(), - ModelID: modelID, - HookEventName: hooks.EventBeforeLLMCall, - Cwd: r.workingDir, + SessionID: sess.ID, + AgentName: a.Name(), + ModelID: modelID, + ModelCapabilities: caps, + HookEventName: hooks.EventBeforeLLMCall, + Cwd: r.workingDir, } for _, t := range r.transforms { out, err := t.fn(ctx, in, msgs) diff --git a/pkg/runtime/transforms_test.go b/pkg/runtime/transforms_test.go index 2cfc48588d..96ac1aea23 100644 --- a/pkg/runtime/transforms_test.go +++ b/pkg/runtime/transforms_test.go @@ -2,8 +2,11 @@ package runtime import ( + "bytes" "context" "errors" + "log/slog" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -11,7 +14,10 @@ import ( "github.com/docker/docker-agent/pkg/agent" "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/hooks" + "github.com/docker/docker-agent/pkg/model/provider/base" + "github.com/docker/docker-agent/pkg/modelinfo" "github.com/docker/docker-agent/pkg/modelsdev" "github.com/docker/docker-agent/pkg/session" "github.com/docker/docker-agent/pkg/team" @@ -19,9 +25,9 @@ import ( ) // modalityModelStore returns a fixed [modelsdev.Model] regardless of -// the requested ID. Tests configure its Modalities to exercise the -// strip_unsupported_modalities transform's three branches: text-only -// (strip), image-supporting (no-op), and unknown-model (no-op). +// the requested ID. Runstream-level tests configure its Modalities to +// exercise the loop's capability resolution feeding the +// strip_unsupported_modalities transform. type modalityModelStore struct { ModelStore @@ -33,20 +39,6 @@ func (m modalityModelStore) GetModel(_ context.Context, _ modelsdev.ID) (*models return m.model, m.err } -// modalityByIDStore returns a different [modelsdev.Model] depending -// on the requested ID, letting tests prove the transform consulted -// the right ID (via [hooks.Input.ModelID]) rather than recomputing -// it from the agent. -type modalityByIDStore struct { - ModelStore - - models map[string]*modelsdev.Model -} - -func (m modalityByIDStore) GetModel(_ context.Context, id modelsdev.ID) (*modelsdev.Model, error) { - return m.models[id.String()], nil -} - // recordingMsgProvider captures the messages each model call sees so // a test can confirm a transform actually rewrote what reached the // provider (rather than just what the in-memory slice ended up @@ -54,19 +46,39 @@ func (m modalityByIDStore) GetModel(_ context.Context, id modelsdev.ID) (*models type recordingMsgProvider struct { mockProvider - got [][]chat.Message + got [][]chat.Message + baseConfig base.Config } +func (p *recordingMsgProvider) BaseConfig() base.Config { return p.baseConfig } + func (p *recordingMsgProvider) CreateChatCompletionStream(_ context.Context, msgs []chat.Message, _ []tools.Tool) (chat.MessageStream, error) { p.got = append(p.got, append([]chat.Message{}, msgs...)) return p.stream, nil } -// TestStripUnsupportedModalitiesTransform pins the three branches of -// the runtime-shipped transform: a text-only model strips images, a -// multimodal model passes them through, and an unknown model also -// passes them through (the call surfaces any modality mismatch as a -// provider error rather than panicking transform-side). +// mixedMediaMsg is a user message carrying every strippable media kind +// (legacy image URL, audio document, video document) plus text and a +// PDF document that the transform must never touch. +func mixedMediaMsg() chat.Message { + return chat.Message{ + Role: chat.MessageRoleUser, + MultiContent: []chat.MessagePart{ + {Type: chat.MessagePartTypeText, Text: "look at this"}, + {Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.wav", MimeType: "audio/wav", Source: chat.DocumentSource{InlineData: []byte{0x52}}}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "clip.mp4", MimeType: "video/mp4", Source: chat.DocumentSource{InlineData: []byte{0x00}}}}, + {Type: chat.MessagePartTypeDocument, Document: &chat.Document{Name: "report.pdf", MimeType: "application/pdf", Source: chat.DocumentSource{InlineData: []byte{0x25}}}}, + {Type: chat.MessagePartTypeText, Text: "and this"}, + }, + } +} + +// TestStripUnsupportedModalitiesTransform pins the capability matrix of +// the runtime-shipped transform. The transform consumes the resolved +// capability set from [hooks.Input.ModelCapabilities]; it never queries +// models.dev itself (the runtime is built with a deliberately +// contradictory multimodal store to prove that). func TestStripUnsupportedModalitiesTransform(t *testing.T) { t.Parallel() @@ -74,95 +86,112 @@ func TestStripUnsupportedModalitiesTransform(t *testing.T) { a := agent.New("root", "instructions", agent.WithModel(prov)) tm := team.New(team.WithAgents(a)) - imgMsg := chat.Message{ - Role: chat.MessageRoleUser, - MultiContent: []chat.MessagePart{ - {Type: chat.MessagePartTypeText, Text: "look at this"}, - {Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, - }, + capsPtr := func(image, pdf, audio, video bool) *modelinfo.ModelCapabilities { + mc := modelinfo.CapsWith(image, pdf, audio, video) + return &mc } cases := []struct { - name string - store modalityModelStore - modelID string - wantStrip bool + name string + caps *modelinfo.ModelCapabilities + // wantMime lists the expected surviving MultiContent parts, in + // order, identified by text or MIME/kind. + wantParts []string }{ - {name: "text-only model strips images", modelID: "test/text", store: modalityModelStore{model: &modelsdev.Model{Modalities: modelsdev.Modalities{Input: []string{"text"}}}}, wantStrip: true}, - {name: "multimodal model passes through", modelID: "test/multimodal", store: modalityModelStore{model: &modelsdev.Model{Modalities: modelsdev.Modalities{Input: []string{"text", "image"}}}}}, - {name: "nil model passes through", modelID: "test/unknown", store: modalityModelStore{model: nil}}, - {name: "lookup error passes through", modelID: "test/unknown", store: modalityModelStore{err: errors.New("not found")}}, - {name: "empty modalities passes through", modelID: "test/empty", store: modalityModelStore{model: &modelsdev.Model{}}}, - {name: "empty ModelID passes through", modelID: "", store: modalityModelStore{model: &modelsdev.Model{Modalities: modelsdev.Modalities{Input: []string{"text"}}}}}, + { + name: "all media capabilities on retains everything", + caps: capsPtr(true, true, true, true), + wantParts: []string{"text", "image", "audio", "video", "pdf", "text"}, + }, + { + name: "text-only strips image, audio, and video but keeps text and pdf", + caps: capsPtr(false, false, false, false), + wantParts: []string{"text", "pdf", "text"}, + }, + { + name: "audio-only override keeps audio, strips image and video", + caps: capsPtr(false, false, true, false), + wantParts: []string{"text", "audio", "pdf", "text"}, + }, + { + name: "unknown model resolves to conservative caps upstream and strips", + caps: &modelinfo.ModelCapabilities{}, + wantParts: []string{"text", "pdf", "text"}, + }, + { + name: "nil capabilities pass messages through untouched", + caps: nil, + wantParts: []string{"text", "image", "audio", "video", "pdf", "text"}, + }, } + // The store contradicts every restrictive case: if the transform + // consulted models.dev instead of the resolved caps, nothing would + // ever be stripped. + store := modalityModelStore{model: &modelsdev.Model{ + Modalities: modelsdev.Modalities{Input: []string{"text", "image", "audio", "video", "pdf"}}, + }} + for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - r, err := NewLocalRuntime(t.Context(), tm, WithModelStore(tc.store)) + t.Parallel() + r, err := NewLocalRuntime(t.Context(), tm, WithModelStore(store)) require.NoError(t, err) got, err := r.stripUnsupportedModalitiesTransform(t.Context(), - &hooks.Input{ModelID: tc.modelID}, []chat.Message{imgMsg}) + &hooks.Input{ModelID: "test/model", ModelCapabilities: tc.caps}, + []chat.Message{mixedMediaMsg()}) require.NoError(t, err) require.Len(t, got, 1) - if tc.wantStrip { - require.Len(t, got[0].MultiContent, 1, "image part must be stripped") - assert.Equal(t, chat.MessagePartTypeText, got[0].MultiContent[0].Type) - } else { - assert.Equal(t, imgMsg, got[0], "messages must reach the model untouched") + + var kinds []string + for _, p := range got[0].MultiContent { + switch { + case p.Type == chat.MessagePartTypeText: + kinds = append(kinds, "text") + case p.Type == chat.MessagePartTypeImageURL: + kinds = append(kinds, "image") + case p.Document != nil && p.Document.MimeType == "application/pdf": + kinds = append(kinds, "pdf") + case p.Document != nil: + kinds = append(kinds, strings.SplitN(p.Document.MimeType, "/", 2)[0]) + } } + assert.Equal(t, tc.wantParts, kinds, "surviving parts (and their order) must match") }) } } -// TestStripUnsupportedModalitiesTransform_UsesInputModelID pins the -// fix for an alloy-mode / per-tool-override correctness bug: the -// transform must trust [hooks.Input.ModelID] (populated by the loop -// with the model it actually picked) and NOT recompute the model by -// calling agent.Model() — doing so would re-randomize the alloy -// pick and miss any per-tool override the loop had applied. +// TestStripUnsupportedModalitiesTransform_EmitsDebugLog verifies each +// stripped part is reported at Debug level with its media kind and a +// reason, so operators can trace why content never reached the model. // -// The test wires a store that reports text-only for one ID and -// multimodal for another. Querying by the text-only ID must strip; -// querying by the multimodal ID must pass through. The agent's own -// model (its pool) is irrelevant — it's never consulted. -func TestStripUnsupportedModalitiesTransform_UsesInputModelID(t *testing.T) { - t.Parallel() +// It swaps the default slog logger and is deliberately NOT parallel so +// no other test logs into the buffer concurrently. +func TestStripUnsupportedModalitiesTransform_EmitsDebugLog(t *testing.T) { + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))) + t.Cleanup(func() { slog.SetDefault(prev) }) - prov := &mockProvider{id: "test/agent-pool-model", stream: &mockStream{}} + prov := &mockProvider{id: "test/model", stream: &mockStream{}} a := agent.New("root", "instructions", agent.WithModel(prov)) tm := team.New(team.WithAgents(a)) - - store := modalityByIDStore{models: map[string]*modelsdev.Model{ - "text/only": {Modalities: modelsdev.Modalities{Input: []string{"text"}}}, - "multi/modal": {Modalities: modelsdev.Modalities{Input: []string{"text", "image"}}}, - "test/agent-pool-model": {Modalities: modelsdev.Modalities{Input: []string{"text", "image"}}}, - }} - r, err := NewLocalRuntime(t.Context(), tm, WithModelStore(store)) + r, err := NewLocalRuntime(t.Context(), tm, WithModelStore(mockModelStore{})) require.NoError(t, err) - imgMsg := chat.Message{ - Role: chat.MessageRoleUser, - MultiContent: []chat.MessagePart{ - {Type: chat.MessagePartTypeText, Text: "describe"}, - {Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, - }, - } - - // ModelID = text-only — strip must happen even though the agent's - // pool model is multimodal. - stripped, err := r.stripUnsupportedModalitiesTransform(t.Context(), - &hooks.Input{ModelID: "text/only"}, []chat.Message{imgMsg}) + textOnly := modelinfo.CapsWith(false, false, false, false) + _, err = r.stripUnsupportedModalitiesTransform(t.Context(), + &hooks.Input{ModelID: "test/model", ModelCapabilities: &textOnly}, + []chat.Message{mixedMediaMsg()}) require.NoError(t, err) - require.Len(t, stripped[0].MultiContent, 1, "image must be stripped when ModelID is text-only") - assert.Equal(t, chat.MessagePartTypeText, stripped[0].MultiContent[0].Type) - // ModelID = multimodal — strip must NOT happen even if some other - // model in scope is text-only. Proves the lookup keys off ModelID. - passed, err := r.stripUnsupportedModalitiesTransform(t.Context(), - &hooks.Input{ModelID: "multi/modal"}, []chat.Message{imgMsg}) - require.NoError(t, err) - assert.Equal(t, imgMsg, passed[0], "images must reach a multimodal ModelID untouched") + logged := buf.String() + for _, kind := range []string{"image", "audio", "video"} { + assert.Contains(t, logged, "kind="+kind, "stripped %s part must be logged with its kind", kind) + assert.Contains(t, logged, "model does not support "+kind+" input", + "stripped %s part must be logged with a reason", kind) + } } // path: a runtime with no registered transforms returns the input @@ -183,7 +212,7 @@ func TestApplyBeforeLLMCallTransforms_NoTransformsIsCheap(t *testing.T) { sess := session.New(session.WithUserMessage("hi")) msgs := []chat.Message{{Role: chat.MessageRoleUser, Content: "hi"}} - got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "", msgs) + got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "", nil, msgs) assert.Equal(t, msgs, got) } @@ -217,7 +246,7 @@ func TestApplyBeforeLLMCallTransforms_OrderAndChain(t *testing.T) { require.NoError(t, err) sess := session.New(session.WithUserMessage("hi")) - got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "test/mock-model", + got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "test/mock-model", nil, []chat.Message{{Role: chat.MessageRoleUser, Content: "hi"}}) require.Len(t, calls, 2, "expected tag_a + tag_b to fire exactly once each") @@ -259,7 +288,7 @@ func TestApplyBeforeLLMCallTransforms_ErrorsAreSwallowed(t *testing.T) { require.NoError(t, err) sess := session.New(session.WithUserMessage("hi")) - got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "test/mock-model", + got := r.applyBeforeLLMCallTransforms(t.Context(), sess, a, "test/mock-model", nil, []chat.Message{{Role: chat.MessageRoleUser, Content: "hi"}}) var contents []string @@ -308,6 +337,115 @@ func TestRunStream_StripsImagesForTextOnlyModel(t *testing.T) { } } +// capsOverrideProvider is a recording provider whose BaseConfig declares +// an explicit `capabilities:` override, mimicking a model config with a +// capabilities block. +type capsOverrideProvider struct { + recordingMsgProvider + + caps *latest.CapabilitiesConfig +} + +func (p *capsOverrideProvider) BaseConfig() base.Config { + return base.Config{ModelConfig: latest.ModelConfig{Capabilities: p.caps}} +} + +// TestRunStream_CapabilityOverrideWinsOverModelsDev is the end-to-end +// regression test for the Step 3 override contract: a model that +// models.dev catalogues as text-only but whose config declares +// `capabilities.image: true` must NOT have its images stripped — the +// loop resolves capabilities with the override applied and the +// transform consumes that result instead of querying models.dev. +func TestRunStream_CapabilityOverrideWinsOverModelsDev(t *testing.T) { + t.Parallel() + + stream := newStreamBuilder().AddContent("ok").AddStopWithUsage(1, 1).Build() + prov := &capsOverrideProvider{ + recordingMsgProvider: recordingMsgProvider{mockProvider: mockProvider{id: "custom/vision", stream: stream}}, + caps: &latest.CapabilitiesConfig{Image: true}, + } + + a := agent.New("root", "instructions", agent.WithModel(prov)) + tm := team.New(team.WithAgents(a)) + + // models.dev claims text-only — without the override the image would + // be stripped (see TestRunStream_StripsImagesForTextOnlyModel). + store := modalityModelStore{model: &modelsdev.Model{ + Modalities: modelsdev.Modalities{Input: []string{"text"}}, + }} + r, err := NewLocalRuntime(t.Context(), tm, WithSessionCompaction(false), WithModelStore(store)) + require.NoError(t, err) + + sess := session.New() + sess.AddMessage(session.UserMessage("", + chat.MessagePart{Type: chat.MessagePartTypeText, Text: "describe"}, + chat.MessagePart{Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, + )) + + for range r.RunStream(t.Context(), sess) { + // drain — only the recorded provider state matters + } + + require.NotEmpty(t, prov.got, "provider must have been called") + var sawImage bool + for _, m := range prov.got[0] { + for _, p := range m.MultiContent { + if p.Type == chat.MessagePartTypeImageURL { + sawImage = true + } + } + } + assert.True(t, sawImage, "explicit capabilities.image override must keep images despite a text-only models.dev record") +} + +func TestRunStream_FallbackUsesItsOwnCapabilities(t *testing.T) { + t.Parallel() + + primary := &failingProvider{id: "custom/text-only", err: errors.New("bad request")} + fallback := &recordingMsgProvider{mockProvider: mockProvider{ + id: "dmr/ai/qwen2.5-vl", + stream: newStreamBuilder().AddContent("ok").AddStopWithUsage(1, 1).Build(), + }} + fallback.baseConfig = base.Config{ModelConfig: latest.ModelConfig{ + Provider: "dmr", + ProviderOpts: map[string]any{ + "supports_images": true, + }, + }} + + a := agent.New("root", "instructions", + agent.WithModel(primary), + agent.WithFallbackModel(fallback), + agent.WithFallbackRetries(0), + ) + tm := team.New(team.WithAgents(a)) + r, err := NewLocalRuntime(t.Context(), tm, WithSessionCompaction(false), WithModelStore(modalityModelStore{})) + require.NoError(t, err) + + sess := session.New() + sess.AddMessage(session.UserMessage("", + chat.MessagePart{Type: chat.MessagePartTypeText, Text: "describe"}, + chat.MessagePart{Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "data:image/png;base64,abc"}}, + )) + + for range r.RunStream(t.Context(), sess) { + } + + require.Len(t, fallback.got, 1) + assert.True(t, hasImagePart(fallback.got[0]), "fallback transforms must use the fallback provider's capabilities") +} + +func hasImagePart(messages []chat.Message) bool { + for _, message := range messages { + for _, part := range message.MultiContent { + if part.Type == chat.MessagePartTypeImageURL || part.Document != nil && strings.HasPrefix(part.Document.MimeType, "image/") { + return true + } + } + } + return false +} + // TestRunStream_TransformErrorDoesNotBreakRun is the end-to-end smoke // test confirming the fail-soft contract: a transform error must not // prevent the model from being called and the run from completing.