Skip to content

Refactor provider registration and implement multiple services - #431

Merged
marcelklehr merged 12 commits into
mainfrom
refactor/providers-per-model
Sep 15, 2026
Merged

marcelklehr merged 12 commits into
mainfrom
refactor/providers-per-model

Conversation

@marcelklehr

@marcelklehr marcelklehr commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

fixes #148
fixes #229

image

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

… at the same time

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
marcelklehr marked this pull request as ready for review September 9, 2026 10:27
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>

@lukasdotcom lukasdotcom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks nice I've tried it out and it works well on my dev instance. A few things that caught my attention while trying it out though:

  • Quota rules seem to apply to all services, but there is still a usage quota for each individual service. Probably worth it to keep that consistent and allow for specifying a service for quota rules too.

  • Model list here seems to need to be reloaded every single time to see all the models (That might be on purpose though).

Image

@julien-nc julien-nc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Amazing, I love it. It works very well, is way more flexible than before and is very nicely done!

A few remarks.

  • When adding the first service, it is expanded. But when adding a service when there are services already, the new service is not expanded. I think this is the intended behaviour. Not a big deal, just in case you want to consistently expand freshly added services.
  • When an integration_openai provider was selected for a task type before upgrading, there is no provider set after upgrading (at least when i did it in my dev env). Not sure if you wanted a transparent migration. It seems possible since the previously selected default model is automatically added as an exposed one. So there is a new candidate provider to replace the old one in the taskprocessing settings.
  • About "detect modalities", I think we need more guidance. We could at least tell the user they have to refresh and select some models after the modalities have been detected.
  • It is a bit confusing that the model lists go away after a page refresh. I understand why it's done like that and I'm not sure this should change. But we could make it easier to understand for users. Maybe changing the button label to "Get model list" is better since it implies we don't have it yet.

@Rikdekker

Rikdekker commented Sep 13, 2026 •

Copy link
Copy Markdown

Tested this branch on a throwaway Nextcloud 35.0.0 RC3 instance, upgrading from the released integration_openai 5.0.0. Two services connected side by side: an existing Mistral config (migrated) and a second OpenAI-compatible endpoint. Short version: the feature works as advertised, and the migration is clean. One usability issue worth considering, below.

What worked

  • Migration from 5.0.0 was lossless. The single url config was converted into service s1 by Version060000Date20260908120000, API key re-encrypted, per-modality flags preserved. Nothing had to be re-entered.
  • Providers are registered per service × model × task type, as intended: 52 providers from 2 services / 3 models (codestral-latest + voxtral-mini-tts-2603 on Mistral, mock-fast + mock-smart on the second service).
  • Mixing providers per task type works, from both the admin UI and occ. Summarize and Generate-a-headline on one backend, Translate on another, everything else on Mistral — and the choice persists across reloads.
  • Provider names (model (service)) make the dropdown readable with multiple backends connected.

Finding: provider ID slugs don't follow their task type IDs

buildProviderId() takes a hardcoded $taskSlug per provider, and those slugs are inconsistent with the task type IDs they serve. Same service, same model:

Provider ID suffix Task type ID
text2text:summary core:text2text:summary
text2text:headline core:text2text:headline
translate core:text2text:translate
improve core:text2text:improve
changetone core:text2text:changetone
reformulate core:text2text:**reformulation**
text2text:emoji core:**generateemoji**

Five of seventeen text task types deviate, and reformulate/reformulation and text2text:emoji/generateemoji deviate in opposite directions.

Why this matters in practice. The admin UI is unaffected — it fills in the IDs itself. But when setting ai.taskprocessing_provider_preferences via occ or a deployment script, it's natural to extrapolate from the majority pattern. I wrote …-mock-smart-text2text:translate (by analogy with …-text2text:summary), and:

  • the value was accepted without any complaint,
  • getPreferredProvider() silently fell back to the first available provider,
  • the AI settings page showed the fallback provider, not an error.

The result is a config that looks applied but silently routes to the wrong backend. I initially concluded the feature was broken before spotting my own typo.

The silent fallback itself lives in core's Manager::getPreferredProvider() and predates this PR — but this PR is what makes it reachable, since before 6.0.0 there was only one provider per task type and nothing to mistype. With multiple backends the failure mode changes from "nothing happens" to "requests go to the wrong provider", which is worse for anyone connecting a local model for privacy reasons.

Deriving the slug from the task type ID (or asserting they match in a test) would remove the class of error entirely. Not a blocker — just cheap to fix now and awkward to change after the IDs are public, since changing them later invalidates stored preferences.

Environment

Nextcloud 35.0.0 RC3 · PHP 8.5 · branch at a8ef1f2 · built with composer install + npm run build on PHP 8.3 / Node 24. Build was clean; the second service was a local mock exposing /v1/models and /v1/chat/completions, since the machine running the test instance cannot reach the real Ollama host.

image image

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr

Copy link
Copy Markdown
Member Author
  • Model list is fetched upon service panel expansion
  • Quota rules are now per service

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
marcelklehr force-pushed the refactor/providers-per-model branch from 5eb895c to f438f02 Compare September 14, 2026 11:33
…rovider IDs

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr

marcelklehr commented Sep 14, 2026 •

Copy link
Copy Markdown
Member Author
  • Core taskprocessing preferred provider setting is now migrated correctly
  • Provider IDs are now deterministic and predictable by the task type they implement

@marcelklehr

Copy link
Copy Markdown
Member Author

We could at least tell the user they have to refresh and select some models after the modalities have been detected.

Refresh is now implicit upon page load / service expansion in the UI. Shall we just select a default model for the user in the same step?

@julien-nc julien-nc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

💥

@marcelklehr

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

…r-model

# Conflicts:
#	lib/TaskProcessing/EmojiProvider.php
#	lib/TaskProcessing/HeadlineProvider.php

Co-authored-by: marcelklehr <986878+marcelklehr@users.noreply.github.com>

Copilot AI commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main into this branch and fixing the two conflicts in EmojiProvider and HeadlineProvider in commit 0bb1aa7.

@marcelklehr
marcelklehr merged commit c7f1e53 into main Sep 15, 2026
25 checks passed
@marcelklehr
marcelklehr deleted the refactor/providers-per-model branch September 15, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show service name in Personal Settings Multiple AI Service Provider Support

5 participants