Skip to content

Commit d999834

Browse files
CybotTMCybotTM
authored
fix(provider): let vision() and embed() use the default configuration (#859)
Fixes #851: `vision()` and `embed()` now resolve the backend-managed default configuration when the caller pins no provider, which `chat()` has done since ADR-034. **What was happening.** Both handed the provider key straight to the registry, so a `null` key threw `No provider specified and no default provider configured` — on installations that have a perfectly good default. That is the usage the feature services invite: options without a provider, because model selection is nr-llm's job rather than the caller's. **Two user-visible failures on typo3-demo came from this one gap.** An image upload in the AI-chat module answered `HTTP 500`, because `mfd/ai-filemetadata` generates an alt text inside the upload request and nr-llm-compat routes that through `VisionServiceInterface`. And with vision resolution failing, `getProviderCapabilities()` reported `visionSupported: false`, so a *selected* image was never expanded into the message and the assistant answered that it saw no image. The instance log carries the exception with `request_url = /typo3/ajax/ai-chat/file-upload`. **What the fix does, and what it deliberately does not.** The resolved configuration drives the call rather than merely unblocking it: its model reaches the provider (which would otherwise fall back to its own hardcoded default), and the pipeline context carries the real configuration instead of a synthesised ad-hoc one, so budget and telemetry attribute the call to it. The caller keeps precedence — an explicitly pinned provider skips the resolution entirely, and a model named in the options is left alone. With no default configuration, or one that is model-less or access-restricted, the call still refuses: the resolver's existing guards apply unchanged, so this resolves a default rather than inventing one. `embed()` had the identical asymmetry and is fixed in the same commit; the issue asked for the siblings to be checked rather than only the reported one. The specialized services (image, speech, DeepL) resolve differently and are not touched here — their attribution gap is #844. **Tests.** Three new cases: the default configuration is used and its model arrives at the provider; a caller-named model survives; and with no default configuration the call still throws. Watched failing before being kept — removing the fallback again reproduces the demo's exception verbatim in two of them. Gates run locally: cgl, PHPStan level 10, the full unit suite (7288 tests), and the changelog check. CI covers the eight-cell matrix. _Assisted by claude-code:claude-fable-5 — [Session](https://claude.ai/code/session_0144iD1P22LotW8rxmxrNGro)_
2 parents 0c33182 + b4c7a11 commit d999834

3 files changed

Lines changed: 178 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
88

9+
### Fixed
10+
11+
- **`vision()` and `embed()` use the default configuration when no provider is pinned**, as `chat()` has since ADR-034. Handing them no provider used to throw "No provider specified and no default provider configured" on an installation that has a perfectly good default — which is what the feature services invite, since model selection is nr-llm's job. The resolved configuration drives the call (its model reaches the provider) and shows up in telemetry as itself rather than as an ad-hoc entry; a provider or model the caller named wins, and with no default configuration the call still refuses rather than picking one (#851).
12+
913
### Changed
1014

1115
- **An MCP tool whose schema uses a union now imports.** `anyOf`, `oneOf`, `allOf` and `not` are carried to the provider verbatim instead of causing the whole tool to be skipped. The rule they were caught by protects against *dropping* a constraint — which would let a model produce arguments the server rejects — and carrying one unchanged does not do that. References (`$ref`, `$defs`) and the draft-2019/2020 applicators (`if`/`then`/`else`, `dependentRequired`, `unevaluatedProperties`, …) stay refused: the first would arrive dangling because the definition block is not carried, the second have no dependable provider support. Concretely, DeepWiki's `ask_question` imports now (#848).

‎Classes/Service/LlmServiceManager.php‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,7 @@ public function embed(string|array $input, ?EmbeddingOptions $options = null): E
619619
{
620620
$options ??= new EmbeddingOptions();
621621
[$providerKey, $optionsArray] = $this->splitProviderKey($options->toArray());
622+
[$configuration, $providerKey, $optionsArray] = $this->applyDefaultConfiguration($providerKey, $optionsArray);
622623

623624
// Cache metadata: EmbedCacheKeyBuilder returns an empty array when
624625
// cache_ttl <= 0 (the EmbeddingOptions::noCache() contract), so the key
@@ -640,7 +641,7 @@ public function embed(string|array $input, ?EmbeddingOptions $options = null): E
640641
$raw = $this->pipeline->run(
641642
ProviderCallContext::forConfiguration(
642643
ProviderOperation::Embedding,
643-
$this->synthesizeTransientConfiguration(ProviderOperation::Embedding, $providerKey),
644+
$configuration ?? $this->synthesizeTransientConfiguration(ProviderOperation::Embedding, $providerKey),
644645
$metadata,
645646
),
646647
function () use ($input, $optionsArray, $providerKey): array {
@@ -681,6 +682,7 @@ public function vision(array $content, ?VisionOptions $options = null): VisionRe
681682
{
682683
$options ??= new VisionOptions();
683684
[$providerKey, $optionsArray] = $this->splitProviderKey($options->toArray());
685+
[$configuration, $providerKey, $optionsArray] = $this->applyDefaultConfiguration($providerKey, $optionsArray);
684686

685687
$normalisedContent = array_values(array_map(
686688
static function (VisionContent|array $item): VisionContent {
@@ -711,7 +713,7 @@ function (VisionContent $item): VisionContent {
711713
);
712714

713715
return $this->runThroughPipeline(
714-
$this->synthesizeTransientConfiguration(ProviderOperation::Vision, $providerKey),
716+
$configuration ?? $this->synthesizeTransientConfiguration(ProviderOperation::Vision, $providerKey),
715717
ProviderOperation::Vision,
716718
function () use ($normalisedContent, $optionsArray, $providerKey): VisionResponse {
717719
$provider = $this->getProvider($providerKey);
@@ -1356,6 +1358,58 @@ private function splitProviderKey(array $optionsArray): array
13561358
return [$providerKey, $optionsArray];
13571359
}
13581360

1361+
/**
1362+
* Let the backend-managed default configuration drive an entry point that
1363+
* takes no configuration argument.
1364+
*
1365+
* `chat()` has done this since ADR-034: with no provider pinned it resolves
1366+
* the default configuration rather than handing `null` to the provider
1367+
* registry, which throws. `vision()` and `embed()` did hand it `null`, so
1368+
* the usage the feature services invite — options without a provider,
1369+
* because model selection is nr-llm's job — failed with "No provider
1370+
* specified and no default provider configured" on an installation that
1371+
* has a perfectly good default. nr-llm-compat's ai_filemetadata bridge is
1372+
* exactly that caller, and an image upload on a site running it died with
1373+
* a 500 rather than a missing alt text.
1374+
*
1375+
* The caller's own choices win: an explicitly pinned provider skips this
1376+
* entirely (the resolver returns null for a non-null key), and a model
1377+
* named in the options is left alone.
1378+
*
1379+
* @param array<string, mixed> $optionsArray
1380+
*
1381+
* @return array{0: LlmConfiguration|null, 1: string|null, 2: array<string, mixed>}
1382+
* the resolved configuration (null when none applies), the provider key to use, and the options
1383+
*/
1384+
private function applyDefaultConfiguration(?string $providerKey, array $optionsArray): array
1385+
{
1386+
if ($providerKey !== null) {
1387+
return [null, $providerKey, $optionsArray];
1388+
}
1389+
1390+
$configuration = $this->configurationResolver->resolveDefaultConfiguration(null);
1391+
if (!$configuration instanceof LlmConfiguration) {
1392+
return [null, null, $optionsArray];
1393+
}
1394+
1395+
// The resolver only returns a configuration that HAS a model, so both
1396+
// lookups below are answered — but a model without a provider record
1397+
// would leave the key null, and then the registry throws as before
1398+
// rather than this method inventing a provider.
1399+
$model = $configuration->getLlmModel();
1400+
$identifier = $model?->getProvider()?->getIdentifier();
1401+
if ($identifier === null || $identifier === '') {
1402+
return [null, null, $optionsArray];
1403+
}
1404+
1405+
$modelId = $model?->getModelId() ?? '';
1406+
if ($modelId !== '' && ($optionsArray['model'] ?? null) === null) {
1407+
$optionsArray['model'] = $modelId;
1408+
}
1409+
1410+
return [$configuration, $identifier, $optionsArray];
1411+
}
1412+
13591413
/**
13601414
* Build a transient LlmConfiguration for direct (ad-hoc) provider calls.
13611415
*

‎Tests/Unit/Service/LlmServiceManagerTest.php‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Netresearch\NrLlm\Domain\Model\EmbeddingResponse;
1919
use Netresearch\NrLlm\Domain\Model\LlmConfiguration;
2020
use Netresearch\NrLlm\Domain\Model\Model;
21+
use Netresearch\NrLlm\Domain\Model\Provider;
2122
use Netresearch\NrLlm\Domain\Model\UsageStatistics;
2223
use Netresearch\NrLlm\Domain\Model\VisionResponse;
2324
use Netresearch\NrLlm\Domain\Repository\LlmConfigurationRepository;
@@ -2066,6 +2067,114 @@ public function chatBypassesDefaultConfigurationWhenProviderPinned(): void
20662067
self::assertInstanceOf(CompletionResponse::class, $result);
20672068
}
20682069

2070+
#[Test]
2071+
public function visionUsesTheDefaultConfigurationWhenNoProviderIsPinned(): void
2072+
{
2073+
// The regression this exists for: a caller that names no provider —
2074+
// which is what the feature services invite, since model selection is
2075+
// nr-llm's job — used to hand null to the provider registry and get
2076+
// "No provider specified and no default provider configured" on an
2077+
// installation that has a perfectly good default. nr-llm-compat's
2078+
// ai_filemetadata bridge is that caller, and an image upload on a site
2079+
// running it answered HTTP 500.
2080+
$provider = new TestableVisionProvider();
2081+
2082+
$providerRecord = self::createStub(Provider::class);
2083+
$providerRecord->method('getIdentifier')->willReturn('openai-vision');
2084+
2085+
$model = self::createStub(Model::class);
2086+
$model->method('getProvider')->willReturn($providerRecord);
2087+
$model->method('getModelId')->willReturn('gpt-5.2-vision');
2088+
2089+
$config = self::createStub(LlmConfiguration::class);
2090+
$config->method('getLlmModel')->willReturn($model);
2091+
$config->method('hasAccessRestrictions')->willReturn(false);
2092+
2093+
$configRepo = $this->createMock(LlmConfigurationRepository::class);
2094+
$configRepo->method('findDefault')->willReturn($config);
2095+
2096+
$manager = $this->createLlmServiceManager(
2097+
$this->extensionConfigStub,
2098+
$this->loggerStub,
2099+
$this->adapterRegistryStub,
2100+
$this->emptyMiddlewarePipeline(),
2101+
self::createStub(CacheManagerInterface::class),
2102+
$configRepo,
2103+
);
2104+
$manager->registerProvider($provider);
2105+
2106+
$result = $manager->vision([['type' => 'text', 'text' => 'What is on this image?']]);
2107+
2108+
self::assertInstanceOf(VisionResponse::class, $result);
2109+
2110+
// The configuration drives the call rather than merely unblocking it:
2111+
// its model reaches the provider, which would otherwise fall back to
2112+
// its own hardcoded default.
2113+
self::assertSame('gpt-5.2-vision', $provider->capturedVisionOptions['model'] ?? null);
2114+
}
2115+
2116+
#[Test]
2117+
public function visionKeepsAModelTheCallerNamedItself(): void
2118+
{
2119+
$provider = new TestableVisionProvider();
2120+
2121+
$providerRecord = self::createStub(Provider::class);
2122+
$providerRecord->method('getIdentifier')->willReturn('openai-vision');
2123+
2124+
$model = self::createStub(Model::class);
2125+
$model->method('getProvider')->willReturn($providerRecord);
2126+
$model->method('getModelId')->willReturn('gpt-5.2-vision');
2127+
2128+
$config = self::createStub(LlmConfiguration::class);
2129+
$config->method('getLlmModel')->willReturn($model);
2130+
$config->method('hasAccessRestrictions')->willReturn(false);
2131+
2132+
$configRepo = $this->createMock(LlmConfigurationRepository::class);
2133+
$configRepo->method('findDefault')->willReturn($config);
2134+
2135+
$manager = $this->createLlmServiceManager(
2136+
$this->extensionConfigStub,
2137+
$this->loggerStub,
2138+
$this->adapterRegistryStub,
2139+
$this->emptyMiddlewarePipeline(),
2140+
self::createStub(CacheManagerInterface::class),
2141+
$configRepo,
2142+
);
2143+
$manager->registerProvider($provider);
2144+
2145+
$manager->vision(
2146+
[['type' => 'text', 'text' => 'What is on this image?']],
2147+
new VisionOptions(model: 'a-model-the-caller-picked'),
2148+
);
2149+
2150+
self::assertSame('a-model-the-caller-picked', $provider->capturedVisionOptions['model'] ?? null);
2151+
}
2152+
2153+
#[Test]
2154+
public function visionStillThrowsWhenThereIsNoDefaultConfigurationEither(): void
2155+
{
2156+
// The fallback resolves a default, it does not invent one: with no
2157+
// default configuration and no pinned provider the call must still say
2158+
// so rather than picking a provider silently (ADR-034).
2159+
$configRepo = $this->createMock(LlmConfigurationRepository::class);
2160+
$configRepo->method('findDefault')->willReturn(null);
2161+
2162+
$manager = $this->createLlmServiceManager(
2163+
$this->extensionConfigStub,
2164+
$this->loggerStub,
2165+
$this->adapterRegistryStub,
2166+
$this->emptyMiddlewarePipeline(),
2167+
self::createStub(CacheManagerInterface::class),
2168+
$configRepo,
2169+
);
2170+
$manager->registerProvider($this->provider);
2171+
2172+
$this->expectException(ProviderException::class);
2173+
$this->expectExceptionMessage('No provider specified and no default provider configured');
2174+
2175+
$manager->vision([['type' => 'text', 'text' => 'test']]);
2176+
}
2177+
20692178
#[Test]
20702179
public function chatThrowsWhenNoDefaultConfigurationAndNoProviderPinned(): void
20712180
{
@@ -2917,6 +3026,14 @@ class TestableProvider extends AbstractProvider
29173026
*/
29183027
public array $capturedMessages = [];
29193028

3029+
/**
3030+
* The options the last analyzeImage() call was handed — the caller's own,
3031+
* plus whatever the manager resolved on their behalf.
3032+
*
3033+
* @var array<string, mixed>
3034+
*/
3035+
public array $capturedVisionOptions = [];
3036+
29203037
public function __construct(
29213038
private readonly string $id = 'openai',
29223039
private readonly string $providerName = 'OpenAI',
@@ -3045,6 +3162,7 @@ public function setNextVisionResponse(VisionResponse $response): void
30453162
public function analyzeImage(array $content, array $options = []): VisionResponse
30463163
{
30473164
$this->capturedContent = $content;
3165+
$this->capturedVisionOptions = $options;
30483166

30493167
return $this->nextVisionResponse ?? new VisionResponse(
30503168
description: 'Default description',

0 commit comments

Comments
 (0)