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(), + ); + } }