Skip to content

fix(provider): let vision() and embed() use the default configuration - #859

Merged
CybotTM merged 1 commit into
mainfrom
fix/vision-default-configuration
Aug 21, 2026
Merged

fix(provider): let vision() and embed() use the default configuration#859
CybotTM merged 1 commit into
mainfrom
fix/vision-default-configuration

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 21, 2026

Copy link
Copy Markdown
Member

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

chat() has resolved the backend-managed default configuration since
ADR-034 when no provider is pinned. vision() and embed() handed the
provider key straight to the registry instead, so null threw:

    No provider specified and no default provider configured

That is the usage the feature services invite — options without a
provider, because model selection is nr-llm's job — and it failed on
installations that have a perfectly good default. nr-llm-compat's
ai_filemetadata bridge is exactly that caller: on typo3-demo an image
upload answered HTTP 500 with this exception, because the alt-text
listener runs inside the upload request. The same missing fallback also
made the chat module report visionSupported=false, so a selected image
was never expanded and the assistant said it saw no image.

The resolved configuration now drives the call rather than merely
unblocking it: its model reaches the provider, 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, a model named in the options is left alone — and with no
default configuration the call still refuses rather than picking one.

Closes #851.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_0144iD1P22LotW8rxmxrNGro
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
@CybotTM
CybotTM requested a review from a team as a code owner August 21, 2026 05:14
Copilot AI lite review requested due to automatic review settings August 21, 2026 05:14
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test-related changes labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.70%. Comparing base (0c33182) to head (b4c7a11).

Files with missing lines Patch % Lines
Classes/Service/LlmServiceManager.php 94.44% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #859   +/-   ##
=========================================
  Coverage     86.69%   86.70%           
- Complexity     9919     9926    +7     
=========================================
  Files           574      574           
  Lines         32064    32080   +16     
=========================================
+ Hits          27799    27814   +15     
- Misses         4265     4266    +1     
Flag Coverage Δ
unit 65.13% <94.44%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Classes/Service/LlmServiceManager.php 89.64% <94.44%> (+0.13%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@CybotTM

CybotTM commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Self-review: b4c7a11

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 21, 2026
Merged via the queue into main with commit d999834 Aug 21, 2026
95 of 96 checks passed
@CybotTM
CybotTM deleted the fix/vision-default-configuration branch August 21, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation tests Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vision() has no default-provider fallback while chat() does — an image upload on the demo 500s because of it

2 participants