From 8e34301031b48e909c27dbc1e4c4646d99972274 Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Fri, 9 Oct 2026 10:58:58 +0200 Subject: [PATCH] [BUGFIX] Derive Symfony AI bridge capabilities from the catalog A bridge with a static model catalog was granted every AiM capability, so a bridge without any image model listed image generation. Catalog models mapping to no AiM capability, such as text-to-speech models, were treated as unlisted and inherited chat, vision and image generation. Every catalog model is now listed with its mapped capabilities, possibly none, and the provider advertises their union. An unlisted model never inherits image generation, and "provider:*" picks the first enabled configuration whose model supports the requested capability. Resolves: #41 Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 10 ++ .../SymfonyAiCompilerPass.php | 30 ++-- Classes/Domain/Model/AiProviderManifest.php | 29 ++-- Classes/Provider/ProviderResolver.php | 13 +- .../Middleware/FallbackDispatchTest.php | 2 +- .../Fixtures/StaticCatalog/Factory.php | 31 ++++ .../Fixtures/StaticCatalog/ModelCatalog.php | 62 ++++++++ .../Fixtures/UnmappedCatalog/Factory.php | 26 +++ .../Fixtures/UnmappedCatalog/ModelCatalog.php | 46 ++++++ .../StaticModelCatalogCapabilitiesTest.php | 150 ++++++++++++++++++ .../Domain/Model/AiProviderManifestTest.php | 55 +++++++ Tests/Unit/Provider/ProviderResolverTest.php | 68 ++++++++ 12 files changed, 491 insertions(+), 31 deletions(-) create mode 100644 Tests/Unit/DependencyInjection/Fixtures/StaticCatalog/Factory.php create mode 100644 Tests/Unit/DependencyInjection/Fixtures/StaticCatalog/ModelCatalog.php create mode 100644 Tests/Unit/DependencyInjection/Fixtures/UnmappedCatalog/Factory.php create mode 100644 Tests/Unit/DependencyInjection/Fixtures/UnmappedCatalog/ModelCatalog.php create mode 100644 Tests/Unit/DependencyInjection/StaticModelCatalogCapabilitiesTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a42146..b9bb89a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +- **Bugfix "Capabilities of static catalogs"**: A Symfony AI bridge with a static model catalog +advertises only the capabilities its models have (#41). Every bridge was granted all of them, so a +bridge without a single image model was listed with image generation. Catalog models that map to no +AiM capability (e.g. text-to-speech or speech-to-text models) are no longer treated as chat, vision or +image models, a model the catalog does not list never inherits image generation, and `provider:*` +picks the first enabled configuration whose model supports the requested capability instead of the +first enabled one. A configuration using such a model is skipped when resolving by capability, and +fails explicit resolution and the connection test, instead of failing at the remote API. Flush the +caches after updating, so the DI container is rebuilt. + - **Bugfix "Temperature on current models"**: Requests to Claude Opus 4.7 and later, Sonnet 5 and other models that no longer accept sampling parameters failed with "`temperature` is deprecated for this model", because every request carried a temperature whether the caller had asked for one or not. diff --git a/Classes/DependencyInjection/SymfonyAiCompilerPass.php b/Classes/DependencyInjection/SymfonyAiCompilerPass.php index 31c6790..c542333 100644 --- a/Classes/DependencyInjection/SymfonyAiCompilerPass.php +++ b/Classes/DependencyInjection/SymfonyAiCompilerPass.php @@ -67,12 +67,14 @@ final class SymfonyAiCompilerPass implements CompilerPassInterface ]; /** - * All AiM capability interfaces — used as the provider-level default - * when a bridge has no ModelCatalog to read from. + * All AiM capability interfaces, used as the provider-level capabilities + * when the bridge's ModelCatalog yields no models to derive them from + * (dynamic or initially empty catalogs). A static catalog narrows the + * provider to the union of its models' capabilities instead. * - * Note: ImageGenerationCapableInterface is included here (provider-level "this - * bridge family can do it") but AiProviderManifest::hasModelCapability() special-cases - * it so unlisted/dynamic-catalog models don't silently inherit it, see there for why. + * ImageGenerationCapableInterface is included, but + * AiProviderManifest::hasModelCapability() never grants it to a model the + * catalog does not list, see there for why. */ private const ALL_CAPABILITIES = [ VisionCapableInterface::class, @@ -125,7 +127,7 @@ public function process(ContainerBuilder $container): void $bridge['description'], 'tx-aim', $bridge['models'], - self::ALL_CAPABILITIES, + $bridge['capabilities'], $serviceId, new Reference(ContainerInterface::class), $featuresDefinition, @@ -194,7 +196,7 @@ private function resolveNamespace(string $packageName): ?string /** * Build a complete bridge definition from a discovered package. * - * @return array{identifier: string, name: string, description: string, factoryClass: string, factoryParam: string, models: array, modelCapabilities: array>, features: array}|null + * @return array{identifier: string, name: string, description: string, factoryClass: string, factoryParam: string, models: array, capabilities: list, modelCapabilities: array>, features: array}|null */ private function buildBridgeDefinition(array $package): ?array { @@ -261,6 +263,11 @@ private function buildBridgeDefinition(array $package): ?array } } + $capabilities = self::ALL_CAPABILITIES; + if ($modelCapabilities !== []) { + $capabilities = array_values(array_unique(array_merge(...array_values($modelCapabilities)))); + } + return [ 'identifier' => $identifier, 'name' => $name, @@ -268,6 +275,7 @@ private function buildBridgeDefinition(array $package): ?array 'factoryClass' => $factoryClass, 'factoryParam' => $factoryParam, 'models' => $models, + 'capabilities' => $capabilities, 'modelCapabilities' => $modelCapabilities, 'features' => $features, ]; @@ -296,11 +304,9 @@ private function extractModelsFromCatalog(array $catalogModels, array $features) // Build display label from model ID $models[$safeModelId] = $this->buildModelLabel($safeModelId, $modelConfig['capabilities'] ?? []); - // Map Symfony AI capabilities to AiM interfaces - $aiCapabilities = $this->mapCapabilities($modelConfig['capabilities'] ?? []); - if ($aiCapabilities !== []) { - $modelCapabilities[$safeModelId] = $aiCapabilities; - } + // Listed even when nothing maps (e.g. text-to-speech), so the model + // is not treated as unlisted and does not inherit provider capabilities. + $modelCapabilities[$safeModelId] = $this->mapCapabilities($modelConfig['capabilities'] ?? []); // Detect features from capabilities foreach ($modelConfig['capabilities'] ?? [] as $cap) { diff --git a/Classes/Domain/Model/AiProviderManifest.php b/Classes/Domain/Model/AiProviderManifest.php index 4e3e669..37a3f80 100644 --- a/Classes/Domain/Model/AiProviderManifest.php +++ b/Classes/Domain/Model/AiProviderManifest.php @@ -58,27 +58,30 @@ public function hasCapability(string $capabilityFqcn): bool /** * Check if a specific model supports a capability. * - * If the model is listed in modelCapabilities, only those capabilities apply. - * Otherwise the model inherits all provider-level capabilities. + * A model listed in modelCapabilities has exactly the capabilities listed + * for it, possibly none. An unlisted model never gets image generation and + * otherwise inherits the provider-level capabilities, minus those only + * specialized catalog models offer. * * @param class-string $capabilityFqcn */ public function hasModelCapability(string $model, string $capabilityFqcn): bool { - // No model-level overrides. All models inherit provider capabilities. - // Exception: image generation needs a genuinely different request contract - // (a dedicated image endpoint, not a chat/messages payload) than every other - // capability here, so it's never safe to assume an unlisted model supports it - // (e.g. a dynamic-catalog bridge like Ollama, which has no image endpoint at all). - // It's only granted when a static ModelCatalog explicitly lists it per model. - if ($this->modelCapabilities === []) { - return $capabilityFqcn !== ImageGenerationCapableInterface::class - && $this->hasCapability($capabilityFqcn); - } - // Model explicitly listed — use only its declared capabilities + // Model explicitly listed: use only its declared capabilities if (isset($this->modelCapabilities[$model])) { return in_array($capabilityFqcn, $this->modelCapabilities[$model], true); } + // Image generation needs a different request contract (a dedicated image + // endpoint, not a chat/messages payload), so an unlisted model is never + // assumed to support it, whether the catalog is discovered at runtime or + // static (e.g. an unknown model on a bridge whose chat model outputs images). + if ($capabilityFqcn === ImageGenerationCapableInterface::class) { + return false; + } + // No model-level data: all models inherit provider capabilities + if ($this->modelCapabilities === []) { + return $this->hasCapability($capabilityFqcn); + } // Model not listed but modelCapabilities is populated: // Inherit all provider capabilities EXCEPT those that are exclusive // to listed models (i.e. capabilities not shared by any listed model diff --git a/Classes/Provider/ProviderResolver.php b/Classes/Provider/ProviderResolver.php index a5544b7..b0f2d4c 100644 --- a/Classes/Provider/ProviderResolver.php +++ b/Classes/Provider/ProviderResolver.php @@ -157,8 +157,10 @@ public function resolveByString(string $notation, string $capabilityFqcn, ?strin } $manifest = $this->registry->getProvider($providerIdentifier); - $checkModel = $model !== '*' ? $model : ''; - if (!$manifest->hasModelCapability($checkModel, $capabilityFqcn)) { + $supported = $model === '*' + ? $manifest->hasCapability($capabilityFqcn) + : $manifest->hasModelCapability($model, $capabilityFqcn); + if (!$supported) { throw new \RuntimeException(sprintf( 'Provider "%s" with model "%s" does not support capability "%s".', $providerIdentifier, @@ -169,16 +171,17 @@ public function resolveByString(string $notation, string $capabilityFqcn, ?strin $configs = $this->configurationRepository->findByProviderIdentifier($providerIdentifier); - // Wildcard model: "openai:*" — use the first enabled config as-is + // Wildcard model "provider:*": the first enabled config whose model supports the capability if ($model === '*') { foreach ($configs as $config) { - if (!$config->disabled) { + if (!$config->disabled && $manifest->hasModelCapability($config->model, $capabilityFqcn)) { return new ResolvedProvider($manifest, $config); } } throw new ProviderNotFoundException(sprintf( - 'No enabled configuration found for provider "%s".', + 'No enabled configuration found for provider "%s" with a model supporting capability "%s".', $providerIdentifier, + $capabilityFqcn, ), 1773874275); } diff --git a/Tests/Functional/Middleware/FallbackDispatchTest.php b/Tests/Functional/Middleware/FallbackDispatchTest.php index 868ad66..8f6057e 100644 --- a/Tests/Functional/Middleware/FallbackDispatchTest.php +++ b/Tests/Functional/Middleware/FallbackDispatchTest.php @@ -177,7 +177,7 @@ public function theRateLimiterCountsOneSlotPerDispatchNotPerFallbackHop(): void #[Test] public function aGovernanceDenialIsNotRetriedAgainstTheWholeChain(): void { - $primary = $this->createConfiguration('primary', ['default' => 1, 'be_groups' => '99']); + $this->createConfiguration('primary', ['default' => 1, 'be_groups' => '99']); $this->createConfiguration('second'); $this->createConfiguration('third'); $this->createBackendUser(); diff --git a/Tests/Unit/DependencyInjection/Fixtures/StaticCatalog/Factory.php b/Tests/Unit/DependencyInjection/Fixtures/StaticCatalog/Factory.php new file mode 100644 index 0000000..3b27f35 --- /dev/null +++ b/Tests/Unit/DependencyInjection/Fixtures/StaticCatalog/Factory.php @@ -0,0 +1,31 @@ +}> + */ + public function getModels(): array + { + return [ + 'chat-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_MESSAGES, + Capability::OUTPUT_TEXT, + Capability::INPUT_IMAGE, + Capability::TOOL_CALLING, + ], + ], + 'embedding-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_TEXT, + Capability::EMBEDDINGS, + ], + ], + 'speech-to-text-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_AUDIO, + Capability::SPEECH_TO_TEXT, + ], + ], + 'text-to-speech-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_TEXT, + Capability::TEXT_TO_SPEECH, + ], + ], + ]; + } +} diff --git a/Tests/Unit/DependencyInjection/Fixtures/UnmappedCatalog/Factory.php b/Tests/Unit/DependencyInjection/Fixtures/UnmappedCatalog/Factory.php new file mode 100644 index 0000000..a047ee6 --- /dev/null +++ b/Tests/Unit/DependencyInjection/Fixtures/UnmappedCatalog/Factory.php @@ -0,0 +1,26 @@ +}> + */ + public function getModels(): array + { + return [ + 'speech-to-text-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_AUDIO, + Capability::SPEECH_TO_TEXT, + ], + ], + 'text-to-speech-model' => [ + 'class' => Model::class, + 'capabilities' => [ + Capability::INPUT_TEXT, + Capability::TEXT_TO_SPEECH, + ], + ], + ]; + } +} diff --git a/Tests/Unit/DependencyInjection/StaticModelCatalogCapabilitiesTest.php b/Tests/Unit/DependencyInjection/StaticModelCatalogCapabilitiesTest.php new file mode 100644 index 0000000..7bf4d6e --- /dev/null +++ b/Tests/Unit/DependencyInjection/StaticModelCatalogCapabilitiesTest.php @@ -0,0 +1,150 @@ +buildBridge('acme/ai-platform', self::FIXTURES . 'StaticCatalog'); + + self::assertIsArray($bridge); + self::assertSame('acme', $bridge['identifier']); + self::assertEqualsCanonicalizing( + [ + VisionCapableInterface::class, + ConversationCapableInterface::class, + TextGenerationCapableInterface::class, + TranslationCapableInterface::class, + ToolCallingCapableInterface::class, + EmbeddingCapableInterface::class, + ], + $bridge['capabilities'], + ); + } + + #[Test] + public function catalogModelsMappingToNoCapabilityAreListedWithoutAny(): void + { + $bridge = $this->buildBridge('acme/ai-platform', self::FIXTURES . 'StaticCatalog'); + + self::assertIsArray($bridge); + self::assertSame([], $bridge['modelCapabilities']['speech-to-text-model'] ?? null); + self::assertSame([], $bridge['modelCapabilities']['text-to-speech-model'] ?? null); + } + + #[Test] + public function speechToTextModelGetsNeitherImageGenerationNorConversation(): void + { + $bridge = $this->buildBridge('acme/ai-platform', self::FIXTURES . 'StaticCatalog'); + self::assertIsArray($bridge); + + $manifest = $this->manifestFor($bridge); + + self::assertFalse($manifest->hasModelCapability('speech-to-text-model', ImageGenerationCapableInterface::class)); + self::assertFalse($manifest->hasModelCapability('speech-to-text-model', ConversationCapableInterface::class)); + } + + /** + * A catalog whose models offer nothing AiM can use leaves the bridge + * without capabilities, rather than falling back to all of them. + */ + #[Test] + public function aCatalogOfOnlyUnmappedModelsGrantsNothing(): void + { + $bridge = $this->buildBridge('acme/ai-voice-platform', self::FIXTURES . 'UnmappedCatalog'); + + self::assertIsArray($bridge); + self::assertSame([], $bridge['capabilities']); + + $manifest = $this->manifestFor($bridge); + $allCapabilities = (new \ReflectionClassConstant(SymfonyAiCompilerPass::class, 'ALL_CAPABILITIES'))->getValue(); + foreach (['speech-to-text-model', 'text-to-speech-model', 'unlisted-model'] as $model) { + foreach ($allCapabilities as $capability) { + self::assertFalse($manifest->hasModelCapability($model, $capability), $model . ' got ' . $capability); + } + } + } + + /** + * Without models to read from, the pass cannot narrow anything down, so the + * bridge keeps the full set. + */ + #[Test] + public function aBridgeWithoutCatalogModelsKeepsAllCapabilities(): void + { + $bridge = $this->buildBridge('symfony/ai-example-platform', self::FIXTURES . 'EmptyCatalog'); + + self::assertIsArray($bridge); + self::assertSame([], $bridge['modelCapabilities']); + self::assertSame( + (new \ReflectionClassConstant(SymfonyAiCompilerPass::class, 'ALL_CAPABILITIES'))->getValue(), + $bridge['capabilities'], + ); + self::assertContains(TextGenerationCapableInterface::class, $bridge['capabilities']); + } + + /** + * @param array $bridge + */ + private function manifestFor(array $bridge): AiProviderManifest + { + return new AiProviderManifest( + identifier: $bridge['identifier'], + name: $bridge['name'], + description: $bridge['description'], + iconIdentifier: 'tx-aim', + supportedModels: $bridge['models'], + capabilities: $bridge['capabilities'], + serviceName: 'test.service', + container: $this->createStub(ContainerInterface::class), + modelCapabilities: $bridge['modelCapabilities'], + ); + } + + /** + * @return array|null + */ + private function buildBridge(string $packageName, string $namespace): ?array + { + $method = new \ReflectionMethod(SymfonyAiCompilerPass::class, 'buildBridgeDefinition'); + + return $method->invoke(new SymfonyAiCompilerPass(), [ + 'name' => $packageName, + 'namespace' => $namespace, + ]); + } +} diff --git a/Tests/Unit/Domain/Model/AiProviderManifestTest.php b/Tests/Unit/Domain/Model/AiProviderManifestTest.php index 4323509..fb38240 100644 --- a/Tests/Unit/Domain/Model/AiProviderManifestTest.php +++ b/Tests/Unit/Domain/Model/AiProviderManifestTest.php @@ -14,6 +14,10 @@ use B13\Aim\Capability\ConversationCapableInterface; use B13\Aim\Capability\ImageGenerationCapableInterface; +use B13\Aim\Capability\TextGenerationCapableInterface; +use B13\Aim\Capability\ToolCallingCapableInterface; +use B13\Aim\Capability\TranslationCapableInterface; +use B13\Aim\Capability\VisionCapableInterface; use B13\Aim\Domain\Model\AiProviderManifest; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -68,4 +72,55 @@ public function staticCatalogModelExplicitlyGrantedImageGenerationStillHasIt(): self::assertTrue($manifest->hasModelCapability('gpt-image-1', ImageGenerationCapableInterface::class)); } + + #[Test] + public function staticCatalogModelListedWithoutCapabilitiesHasNone(): void + { + $manifest = $this->manifest( + [ConversationCapableInterface::class, TextGenerationCapableInterface::class], + [ + 'chat-model' => [ConversationCapableInterface::class, TextGenerationCapableInterface::class], + 'speech-to-text-model' => [], + ], + ); + + self::assertFalse($manifest->hasModelCapability('speech-to-text-model', ConversationCapableInterface::class)); + } + + /** + * A chat model listing image output at the top capability count made image + * generation look like a capability every unlisted model shares. + */ + #[Test] + public function unlistedModelOfStaticCatalogNeverInheritsImageGeneration(): void + { + $manifest = $this->manifestWithImageOutputOnChatModel(); + + self::assertFalse($manifest->hasModelCapability('unlisted-model', ImageGenerationCapableInterface::class)); + } + + #[Test] + public function unlistedModelOfStaticCatalogStillInheritsConversation(): void + { + $manifest = $this->manifestWithImageOutputOnChatModel(); + + self::assertTrue($manifest->hasModelCapability('unlisted-model', ConversationCapableInterface::class)); + } + + private function manifestWithImageOutputOnChatModel(): AiProviderManifest + { + $chat = [ + VisionCapableInterface::class, + ConversationCapableInterface::class, + TextGenerationCapableInterface::class, + ToolCallingCapableInterface::class, + ImageGenerationCapableInterface::class, + TranslationCapableInterface::class, + ]; + + return $this->manifest($chat, [ + 'chat-model' => $chat, + 'image-model' => [ImageGenerationCapableInterface::class], + ]); + } } diff --git a/Tests/Unit/Provider/ProviderResolverTest.php b/Tests/Unit/Provider/ProviderResolverTest.php index 53b690f..483e469 100644 --- a/Tests/Unit/Provider/ProviderResolverTest.php +++ b/Tests/Unit/Provider/ProviderResolverTest.php @@ -18,6 +18,7 @@ use B13\Aim\Domain\Model\ProviderConfiguration; use B13\Aim\Domain\Repository\ProviderConfigurationRepository; use B13\Aim\Domain\Repository\RequestLogRepository; +use B13\Aim\Exception\ProviderNotFoundException; use B13\Aim\Governance\ConfigurationAccess; use B13\Aim\Provider\ProviderResolver; use B13\Aim\Registry\AiProviderRegistry; @@ -129,4 +130,71 @@ public function resolveWithFallbackAcceptsProviderNotationAlongsideConfiguration self::assertSame('anthropic', $resolved->configuration->providerIdentifier); } + + /** + * "provider:*" used to return the first enabled configuration whatever its + * model, so an image request could end up on a chat model. + */ + #[Test] + public function wildcardNotationPicksTheFirstConfigurationWhoseModelSupportsTheCapability(): void + { + $resolver = $this->wildcardResolver([ + ['uid' => 1, 'ai_provider' => 'acme', 'model' => 'chat-model', 'disabled' => 0], + ['uid' => 2, 'ai_provider' => 'acme', 'model' => 'image-model', 'disabled' => 0], + ]); + + self::assertSame('image-model', $resolver->resolveByString('acme:*', ImageGenerationCapableInterface::class)->configuration->model); + self::assertSame('chat-model', $resolver->resolveByString('acme:*', ConversationCapableInterface::class)->configuration->model); + } + + #[Test] + public function wildcardNotationFailsWhenNoConfiguredModelSupportsTheCapability(): void + { + $resolver = $this->wildcardResolver([ + ['uid' => 1, 'ai_provider' => 'acme', 'model' => 'chat-model', 'disabled' => 0], + ]); + + $this->expectException(ProviderNotFoundException::class); + $this->expectExceptionCode(1773874275); + + $resolver->resolveByString('acme:*', ImageGenerationCapableInterface::class); + } + + /** + * @param list> $rows + */ + private function wildcardResolver(array $rows): ProviderResolver + { + $manifest = new AiProviderManifest( + identifier: 'acme', + name: 'Acme', + description: '', + iconIdentifier: '', + supportedModels: [], + capabilities: [ConversationCapableInterface::class, ImageGenerationCapableInterface::class], + serviceName: 'aim.symfony_ai.acme', + container: $this->createStub(ContainerInterface::class), + modelCapabilities: [ + 'chat-model' => [ConversationCapableInterface::class], + 'image-model' => [ImageGenerationCapableInterface::class], + ], + ); + + $registry = $this->createStub(AiProviderRegistry::class); + $registry->method('hasProvider')->willReturn(true); + $registry->method('getProvider')->willReturn($manifest); + + $configurationRepository = $this->createStub(ProviderConfigurationRepository::class); + $configurationRepository->method('findByProviderIdentifier')->willReturn( + array_map(static fn(array $row): ProviderConfiguration => new ProviderConfiguration($row), $rows), + ); + + return new ProviderResolver( + $registry, + $configurationRepository, + $this->createStub(DisabledModelRegistry::class), + $this->createStub(RequestLogRepository::class), + new ConfigurationAccess(), + ); + } }