From b351ac5edb1e66998ed210fc434eb0703058ebeb Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Sat, 22 Aug 2026 18:12:58 +0200 Subject: [PATCH 1/8] [AI-3] Let administrators curate which models are offered Adds a toggle to each model row that hides it from the default-model pickers or puts it back, stored as deactivated_at rather than in active, which the catalogue sync owns and rewrites on every refresh. Curation, not enforcement: anything already pointing at a hidden model keeps resolving, so switching a row off can never silently break a running feature. With a curated list to choose from, the connection form gains the default chat model selector, backed by a contract validation that a designated default must be a model the server actually reported. It is validated only on change, so a catalogue that shrinks underneath a stored selection does not block every unrelated save. Part 7 of the AI-3 stack. https://community.openproject.org/work_packages/66020 --- .../llm_connections/models_row_component.rb | 36 +++++++++- .../llm_connections/models_table_component.rb | 5 +- .../llm_connections/base_contract.rb | 33 +++++++++ .../admin/llm_connections_controller.rb | 2 +- .../admin/llm_models_controller.rb | 14 ++++ app/forms/llm_connections/connection_form.rb | 60 +++++++++++++++++ app/models/llm_connection.rb | 6 ++ app/models/llm_model.rb | 13 ++++ config/locales/en.yml | 12 ++++ config/routes.rb | 1 + ...100000_add_deactivated_at_to_llm_models.rb | 42 ++++++++++++ .../llm_connections/update_contract_spec.rb | 26 +++++++ spec/features/admin/llm_connection_spec.rb | 15 +++++ spec/models/llm_model_deactivation_spec.rb | 67 +++++++++++++++++++ spec/requests/admin/llm_models_spec.rb | 41 +++++++++++- 15 files changed, 367 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20260813100000_add_deactivated_at_to_llm_models.rb create mode 100644 spec/models/llm_model_deactivation_spec.rb diff --git a/app/components/llm_connections/models_row_component.rb b/app/components/llm_connections/models_row_component.rb index 26346b7496d4..ffb15bab2437 100644 --- a/app/components/llm_connections/models_row_component.rb +++ b/app/components/llm_connections/models_row_component.rb @@ -76,12 +76,46 @@ def source end def source_label - return %i[accent source_manual] if llm_model.manual? + # Withdrawn wins over hidden: the withdrawal is why the toggle is inert, + # and an administrator needs that explanation more than their own choice. return %i[attention source_withdrawn] if llm_model.withdrawn? + return %i[attention source_deactivated] if llm_model.deactivated? + return %i[accent source_manual] if llm_model.manual? %i[secondary source_discovered] end + # Whether a feature may choose this model. A withdrawn model has nothing to + # switch on -- the server stopped offering it -- so its toggle is inert + # rather than absent, which keeps the column aligned and says why. + def status + render(Primer::Alpha::ToggleSwitch.new(**toggle_options)) + end + + def toggle_options + options = { + checked: llm_model.selectable?, + enabled: togglable?, + size: :small, + # A bare ToggleSwitch has no accessible name, and axe fails without one. + aria: { label: I18n.t("admin.llm_models.index.toggle_aria_label", model: llm_model.name) }, + test_selector: "llm-model--toggle-#{llm_model.id}" + } + + togglable? ? options.merge(mutation_options) : options + end + + def mutation_options + { + src: url_helpers.toggle_llm_model_path(llm_model), + csrf_token: helpers.form_authenticity_token, + data: { "turbo-method": :post, "turbo-stream": true }, + classes: "op-primer-adjustments__toggle-switch--hidden-loading-indicator" + } + end + + def togglable? = llm_model.active? + def button_links llm_model.manual? ? [edit_link, delete_link] : [edit_link] end diff --git a/app/components/llm_connections/models_table_component.rb b/app/components/llm_connections/models_table_component.rb index cb01b0817d00..94aeac16d1f3 100644 --- a/app/components/llm_connections/models_table_component.rb +++ b/app/components/llm_connections/models_table_component.rb @@ -34,7 +34,7 @@ module LlmConnections # Rendering never issues an HTTP request: the catalogue is refreshed explicitly # through the "Refresh models" action. class ModelsTableComponent < OpPrimer::BorderBoxTableComponent - columns :identifier, :kind, :context_window, :source + columns :identifier, :kind, :context_window, :source, :status main_column :identifier @@ -66,7 +66,8 @@ def headers [:identifier, { caption: I18n.t("admin.llm_models.index.identifier") }], [:kind, { caption: I18n.t("admin.llm_models.index.kind") }], [:context_window, { caption: I18n.t("admin.llm_models.index.context_window") }], - [:source, { caption: I18n.t("admin.llm_models.index.source") }] + [:source, { caption: I18n.t("admin.llm_models.index.source") }], + [:status, { caption: I18n.t("admin.llm_models.index.status") }] ] end diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index 9e1963c346fe..d928bc6aa3bc 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -38,6 +38,8 @@ class BaseContract < ModelContract attribute :api_key attribute :default_chat_model_id attribute :default_embedding_model_id + attribute :default_chat_model_id + attribute :default_embedding_model_id validates :base_url, presence: true validates :api_format, inclusion: { in: Llm::Adapters::FORMATS } @@ -52,14 +54,45 @@ class BaseContract < ModelContract validates :base_url, url: { message: :invalid_url }, unless: -> { base_url.blank? } validate :features_require_connection + validate :default_models_offered_by_server + validate :default_chat_model_can_chat private + # The mirror image of default_embedding_model_can_embed: a model the server + # positively identifies as an embedding model is not a chat candidate. + def default_chat_model_can_chat + llm_model = model.default_chat_model + return if llm_model.blank? + return unless model.changed_attributes.include?("default_chat_model_id") + + embedding = model.capability_verdicts + .for_capability(:embeddings) + .exists?(model_id: llm_model.external_id, state: "supported") + + errors.add(:default_chat_model_id, :cannot_chat) if embedding + end + def features_require_connection return unless model.llm_features_enabled return if model.base_url.present? errors.add :llm_features_enabled, :requires_connection end + + # A designated default must be a model the server actually reported. Validated + # only when it changes, so a catalogue that shrinks underneath a stored + # selection does not block every unrelated save; the dangling state is + # surfaced in the UI instead. + def default_models_offered_by_server + %i[default_chat_model_id default_embedding_model_id].each do |attribute| + value = model.public_send(attribute) + next if value.blank? + next unless model.changed_attributes.include?(attribute.to_s) + next if model.available_model_ids.include?(value) + + errors.add attribute, :not_available + end + end end end diff --git a/app/controllers/admin/llm_connections_controller.rb b/app/controllers/admin/llm_connections_controller.rb index 50061aebde98..302033c60a30 100644 --- a/app/controllers/admin/llm_connections_controller.rb +++ b/app/controllers/admin/llm_connections_controller.rb @@ -123,7 +123,7 @@ def redirect_with_error(message) # saved value, so submitting it unchanged posts an empty string. def llm_connection_params permitted = params.expect( - llm_connection: %i[llm_features_enabled api_format base_url api_key] + llm_connection: %i[llm_features_enabled api_format base_url api_key default_chat_model_id] ) permitted.delete(:api_key) if permitted[:api_key].blank? permitted.to_h.symbolize_keys diff --git a/app/controllers/admin/llm_models_controller.rb b/app/controllers/admin/llm_models_controller.rb index 6e41973a4a19..825a4813288f 100644 --- a/app/controllers/admin/llm_models_controller.rb +++ b/app/controllers/admin/llm_models_controller.rb @@ -124,6 +124,20 @@ def destroy redirect_to llm_models_path, status: :see_other end + # Hides a model from the pickers, or puts it back. Deliberately does not + # touch +active+, which the catalogue sync owns and would overwrite. + def toggle + llm_model = @connection.models.find(params.expect(:id)) + + # A withdrawn model has nothing to switch on; its toggle is rendered + # disabled, and this refuses a request that got here anyway. + return render(json: {}, status: :unprocessable_entity) unless llm_model.active? + + llm_model.update!(deactivated_at: llm_model.deactivated? ? nil : Time.current) + + render json: {}, status: :ok + end + private def set_connection diff --git a/app/forms/llm_connections/connection_form.rb b/app/forms/llm_connections/connection_form.rb index 8669f08b9968..2c0b33eaf05d 100644 --- a/app/forms/llm_connections/connection_form.rb +++ b/app/forms/llm_connections/connection_form.rb @@ -103,6 +103,31 @@ class ConnectionForm < ApplicationForm end end + if models_available? + # An autocompleter rather than a select: a gateway reports hundreds of + # models, and every one of them would otherwise be inlined as an option + # in the page body. decorated: true serialises the list into the element, + # so this needs no endpoint of its own. + f.autocompleter( + name: :default_chat_model_id, + label: LlmConnection.human_attribute_name(:default_chat_model_id), + caption: I18n.t("admin.llm_connections.form.default_chat_model_caption"), + autocomplete_options: { + decorated: true, + inputValue: model.default_chat_model_id, + placeholder: I18n.t("label_none_parentheses") + } + ) do |list| + list.option(label: I18n.t("label_none_parentheses"), value: "", + selected: model.default_chat_model_id.blank?) + + default_chat_model_options.each do |model_id| + list.option(label: option_label(model_id), value: model_id, + selected: model.default_chat_model_id == model_id) + end + end + end + f.submit( name: :submit, label: submit_label, @@ -142,6 +167,41 @@ def server_descriptions ) end + def models_available? + model.available_model_ids.any? + end + + # Deactivated models are hidden, and so is anything positively known to be + # an embedding model -- it is a different kind of model, not a chat choice. + # The one already chosen is kept regardless: dropping it would silently + # blank the field on the next save. + def default_chat_model_options + chat_capable = model.selectable_model_ids.reject { |id| embeddings_state(id) == :supported } + + (chat_capable + [model.default_chat_model_id]).compact_blank.uniq + end + + # The same friendly name the model table shows; the identifier stays the value. + def option_label(model_id) + model_names[model_id].presence || model_id + end + + def model_names + @model_names ||= model.models.pluck(:external_id, :display_name).to_h + end + + # No verdict at all is the same as an inconclusive one: we do not know. + def embeddings_state(model_id) + embeddings_verdicts[model_id]&.to_sym || :unknown + end + + def embeddings_verdicts + @embeddings_verdicts ||= model.capability_verdicts + .for_capability(:embeddings) + .pluck(:model_id, :state) + .to_h + end + def submit_label model.persisted? ? I18n.t(:button_save) : I18n.t("admin.llm_connections.form.button_connect") end diff --git a/app/models/llm_connection.rb b/app/models/llm_connection.rb index 9912b52d0de0..90cc82742ed5 100644 --- a/app/models/llm_connection.rb +++ b/app/models/llm_connection.rb @@ -103,6 +103,12 @@ def models_stale? connection_fingerprint.present? && connection_fingerprint != settings_fingerprint end + # What a picker should offer: the above, minus what an administrator has + # switched off. + def selectable_model_ids + models.selectable.by_identifier.pluck(:external_id) + end + def server_flavour options["server_flavour"].presence&.to_sym end diff --git a/app/models/llm_model.rb b/app/models/llm_model.rb index 7550eff79852..75d9ac73b05f 100644 --- a/app/models/llm_model.rb +++ b/app/models/llm_model.rb @@ -47,6 +47,13 @@ class LlmModel < ApplicationRecord scope :manual, -> { where(manual: true) } scope :by_identifier, -> { order(:external_id) } + # What an administrator is willing to have chosen. Distinct from +active+, + # which the catalogue sync owns and rewrites on every refresh. + scope :deactivated, -> { where.not(deactivated_at: nil) } + scope :selectable, -> { active.where(deactivated_at: nil) } + + def deactivated? = deactivated_at.present? + # Everything that points at a model does so by its identifier string, so a # rename has to carry them along or it silently orphans them. # @@ -69,6 +76,12 @@ def cascade_delete! clear_connection_defaults end + # Offerable in a picker. Note that this is *not* what decides whether a model + # still resolves: a feature already bound to a deactivated model keeps working, + # and is surfaced as a warning instead. Switching a row off must never silently + # break a running feature. + def selectable? = active? && !deactivated? + def name = display_name.presence || external_id def clear_connection_defaults diff --git a/config/locales/en.yml b/config/locales/en.yml index 324e502d8547..a0b3f6e060f9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -211,6 +211,7 @@ en: base_url: "Host URL" # ActiveRecord::Base.human_attribute_name strips the _id suffix, so these # keys deliberately do not carry it (see lib/open_project/patches/active_record_i18n.rb). + default_chat_model: "Default chat model" llm_features_enabled: "Enable LLMs for this instance" llm_model: display_name: "Display name" @@ -701,6 +702,12 @@ en: request_timed_out: "did not respond in time. Please ensure the LLM server is reachable and not overloaded." ssl_error: "could not be reached over a secure connection. Please check the LLM server's TLS certificate, or use an http:// URL if the server does not offer TLS." ssrf_filtered: "resolves to a blocked address. If the LLM server runs on an internal network, allow its IP via the %{env_name} environment variable." + default_chat_model_id: + cannot_chat: "is an embedding model, according to the LLM server, and cannot be used for chat." + not_available: "is not offered by the configured LLM server." + default_embedding_model_id: + cannot_embed: "cannot create embeddings, according to the LLM server." + not_available: "is not offered by the configured LLM server." llm_features_enabled: requires_connection: "cannot be turned on before a connection has been configured." meeting: @@ -1730,6 +1737,7 @@ en: api_key_remove: "Remove key" base_url_caption: "The base URL of the server, for example https://example.com/v1." button_connect: "Connect" + default_chat_model_caption: "Used by AI features that do not select a model themselves." llm_features_enabled_caption: "When turned off, all LLM-backed AI features stop working. The connection settings are kept." label_connecting: "Contacting the LLM server…" server_description: "Connect OpenProject to a server that speaks the %{api_format} API, so AI features can use it. The host URL below decides which server that is." @@ -1780,6 +1788,8 @@ en: kind: "Type" refresh: "Refresh models" source: "Source" + source_deactivated: "Hidden" + source_deactivated_description: "Hidden by an administrator" source_discovered: "Server" source_discovered_description: "Reported by the server" source_manual: "Manual" @@ -1787,6 +1797,8 @@ en: source_withdrawn: "Withdrawn" source_withdrawn_description: "No longer reported by the server" stale_warning: "The connection settings changed after the model list was last refreshed, so the list may be out of date. Refresh the models to be sure." + status: "Available for use" + toggle_aria_label: "Make %{model} available to AI features" new: description: "Name a model this server can use but does not advertise, and describe what it can do." title: "Add a model" diff --git a/config/routes.rb b/config/routes.rb index 3abcb7b0f318..94da62f440ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -757,6 +757,7 @@ member do get :delete_dialog + post :toggle end end diff --git a/db/migrate/20260813100000_add_deactivated_at_to_llm_models.rb b/db/migrate/20260813100000_add_deactivated_at_to_llm_models.rb new file mode 100644 index 000000000000..8f59a81367c9 --- /dev/null +++ b/db/migrate/20260813100000_add_deactivated_at_to_llm_models.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +# Separates "the server stopped offering this" from "an administrator does not +# want this used". +# +# +active+ is owned by the catalogue sync, which sets it on every refresh and +# clears it for models the server no longer reports. An administrator's choice +# cannot live there: the next "Refresh models" would silently undo it, and the +# model would be labelled "no longer reported" when it is merely hidden. +class AddDeactivatedAtToLlmModels < ActiveRecord::Migration[8.1] + def change + add_column :llm_models, :deactivated_at, :datetime + end +end diff --git a/spec/contracts/llm_connections/update_contract_spec.rb b/spec/contracts/llm_connections/update_contract_spec.rb index e0daa331c3a2..8a8225745530 100644 --- a/spec/contracts/llm_connections/update_contract_spec.rb +++ b/spec/contracts/llm_connections/update_contract_spec.rb @@ -201,4 +201,30 @@ expect(models_request).not_to have_been_made end end + + describe "default model selection" do + let(:connection) { create(:llm_connection, :with_models, base_url:) } + + context "with a model the server offers" do + before { connection.default_chat_model_id = "bge-m3" } + + include_examples "contract is valid" + end + + context "with a model the server does not offer" do + before { connection.default_chat_model_id = "not-there" } + + include_examples "contract is invalid", default_chat_model_id: :not_available + end + + context "with a model the server says is an embedding model" do + before do + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + connection.default_chat_model_id = "bge-m3" + end + + include_examples "contract is invalid", default_chat_model_id: :cannot_chat + end + end end diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index f03e0af72b5b..dc07975f0025 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -142,9 +142,24 @@ def choose_action(item) expect(page).to have_test_selector("llm-model--refresh-button") expect(page).to have_text(connection.models.first.external_id) + expect(page).to have_test_selector("llm-model--toggle-#{connection.models.first.id}") expect(page).to be_axe_clean.within("#content") end + it "hides a model from the feature pickers when it is switched off" do + llm_model = connection.models.find_by(external_id: "qwen3.6-27b") + + visit llm_models_path + find_test_selector("llm-model--toggle-#{llm_model.id}").click + + wait_for { llm_model.reload.deactivated_at }.not_to be_nil + + # The toggle acknowledges with JSON rather than re-rendering the row, so + # the source label only catches up on the next load. + visit llm_models_path + expect(page).to have_text("Hidden") + end + # The chat capabilities are hidden client-side, so only a browser shows that # the type choice actually reaches them. it "adds a model by hand and offers the capabilities its type can have" do diff --git a/spec/models/llm_model_deactivation_spec.rb b/spec/models/llm_model_deactivation_spec.rb new file mode 100644 index 000000000000..e75e259c6901 --- /dev/null +++ b/spec/models/llm_model_deactivation_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe LlmModel, "deactivation", :llm_server_helpers, :webmock, + with_flag: { llm_connection: true } do + let(:base_url) { "https://example.com/v1" } + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let(:model) { connection.models.find_by(external_id: "qwen3.6-27b") } + + before { model.update!(deactivated_at: Time.current) } + + it "hides the model from the pickers" do + expect(connection.selectable_model_ids).not_to include("qwen3.6-27b") + expect(connection.selectable_model_ids).to include("bge-m3") + end + + # administrator switches off must never silently break a running feature. + + # The reason deactivated_at exists rather than reusing active: the sync writes + # active on every refresh, so an administrator's choice stored there would be + # undone by the next "Refresh models". + it "survives a catalogue sync that still reports the model" do + mock_llm_models_response(base_url) + + LlmConnections::SyncModelsService.new(connection).call + + expect(model.reload).to be_deactivated + expect(model).to be_active + expect(model).not_to be_selectable + end + + it "is distinct from a model the server withdrew" do + withdrawn = create(:llm_model, :withdrawn, llm_connection: connection, external_id: "gone") + + expect(withdrawn).to be_withdrawn + expect(model).not_to be_withdrawn + end +end diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 36266da677d7..767d3568d8e1 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -177,7 +177,7 @@ 25.times { |n| create(:llm_model, llm_connection: connection, external_id: format("model-%03d", n)) } end - def rendered_rows(body) = body.scan(/model-\d{3}/).uniq.size + def rendered_rows(body) = body.scan("llm-model--toggle-").size it "shows one page of rows at a time rather than every model" do get llm_models_path, params: { per_page: 20 } @@ -204,7 +204,7 @@ def rendered_rows(body) = body.scan(/model-\d{3}/).uniq.size create(:llm_model, llm_connection: connection, external_id: "e5-large", display_name: "BGE compatible") end - def rendered_rows(body) = ["qwen3.6-27b", "bge-m3", "BGE compatible"].count { |name| body.include?(name) } + def rendered_rows(body) = body.scan("llm-model--toggle-").size it "narrows the table to matching models" do get search_llm_models_path, params: { filters: } @@ -584,4 +584,41 @@ def rendered_rows(body) = ["qwen3.6-27b", "bge-m3", "BGE compatible"].count { |n end end end + + describe "POST /admin/llm_models/:id/toggle" do + let!(:connection) { create(:llm_connection, :enabled, base_url:) } + let!(:llm_model) { create(:llm_model, llm_connection: connection, external_id: "qwen3.6-27b") } + + before { login_as admin } + + it "hides the model from the pickers and puts it back" do + post toggle_llm_model_path(llm_model) + + expect(response).to have_http_status(:ok) + expect(llm_model.reload).to be_deactivated + expect(connection.selectable_model_ids).not_to include("qwen3.6-27b") + + post toggle_llm_model_path(llm_model) + + expect(llm_model.reload).not_to be_deactivated + expect(connection.selectable_model_ids).to include("qwen3.6-27b") + end + + it "refuses a model the server has withdrawn" do + withdrawn = create(:llm_model, :withdrawn, llm_connection: connection, external_id: "gone") + + post toggle_llm_model_path(withdrawn) + + expect(response).to have_http_status(:unprocessable_entity) + expect(withdrawn.reload).not_to be_deactivated + end + + it "is refused to a non-admin" do + login_as create(:user) + + post toggle_llm_model_path(llm_model) + + expect(llm_model.reload).not_to be_deactivated + end + end end From 66fc8113cc53fcfa539434a97c509676cab36f89 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Fri, 4 Sep 2026 14:45:00 +0200 Subject: [PATCH 2/8] [AI-3] Keep the Source column to provenance only The Source column answered two questions at once: switching a model off relabelled it "Hidden by administrator" and dropped where it came from, so a model added by hand stopped saying so. The Status toggle right beside it already shows whether a model may be used. Following the UX review with Tom, Source now only answers where a model came from: Added manually, Reported by server, or No longer reported. --- app/components/llm_connections/models_row_component.rb | 3 --- config/locales/en.yml | 2 -- spec/features/admin/llm_connection_spec.rb | 8 ++++++-- spec/requests/admin/llm_models_spec.rb | 9 +++++++++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/components/llm_connections/models_row_component.rb b/app/components/llm_connections/models_row_component.rb index ffb15bab2437..edaf9a633bd9 100644 --- a/app/components/llm_connections/models_row_component.rb +++ b/app/components/llm_connections/models_row_component.rb @@ -76,10 +76,7 @@ def source end def source_label - # Withdrawn wins over hidden: the withdrawal is why the toggle is inert, - # and an administrator needs that explanation more than their own choice. return %i[attention source_withdrawn] if llm_model.withdrawn? - return %i[attention source_deactivated] if llm_model.deactivated? return %i[accent source_manual] if llm_model.manual? %i[secondary source_discovered] diff --git a/config/locales/en.yml b/config/locales/en.yml index a0b3f6e060f9..34624414ae52 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1788,8 +1788,6 @@ en: kind: "Type" refresh: "Refresh models" source: "Source" - source_deactivated: "Hidden" - source_deactivated_description: "Hidden by an administrator" source_discovered: "Server" source_discovered_description: "Reported by the server" source_manual: "Manual" diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index dc07975f0025..1b8d947ecf73 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -155,9 +155,13 @@ def choose_action(item) wait_for { llm_model.reload.deactivated_at }.not_to be_nil # The toggle acknowledges with JSON rather than re-rendering the row, so - # the source label only catches up on the next load. + # the table only catches up on the next load. visit llm_models_path - expect(page).to have_text("Hidden") + + within_test_selector("llm-model--toggle-#{llm_model.id}") do + expect(page).to have_css("button[aria-pressed='false']") + end + expect(page).to have_no_text("Hidden") end # The chat capabilities are hidden client-side, so only a browser shows that diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 767d3568d8e1..e842731e3c8c 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -620,5 +620,14 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(llm_model.reload).not_to be_deactivated end + + it "leaves the source of a hidden model answering where it came from" do + create(:llm_model, :manual, :deactivated, llm_connection: connection, external_id: "by-hand") + + get llm_models_path + + expect(response.body).to include("Added manually by an administrator") + expect(response.body).not_to include("Hidden") + end end end From 0370218eca84fd846525f72d917743e59f94e1f8 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Fri, 4 Sep 2026 15:29:27 +0200 Subject: [PATCH 3/8] [AI-3] Move the default chat model to the AI models page The default chat model was chosen on the LLM settings page, next to the server credentials, while the list it may be chosen from lives on the AI models page. A default may only name a model that was fetched and is switched on, so following the UX review with Tom the picker now sits in a "Default models" section above the model table, saved through its own PATCH /admin/llm_models/defaults action. The picker offers the connection's chat models, keeping whatever is stored so a save cannot silently blank it, and still runs through the update contract. The contract's duplicated attribute declarations are gone and its chat check now asks the model whether it embeds, the same predicate the table and the model form use. --- .../default_models_component.html.erb | 39 ++++++++++ .../default_models_component.rb | 50 ++++++++++++ .../llm_connections/base_contract.rb | 11 +-- .../admin/llm_connections_controller.rb | 2 +- .../admin/llm_models_controller.rb | 15 ++++ app/forms/llm_connections/connection_form.rb | 60 --------------- .../llm_connections/default_models_form.rb | 77 +++++++++++++++++++ app/models/llm_connection.rb | 8 ++ app/views/admin/llm_models/index.html.erb | 2 + config/locales/en.yml | 6 +- config/routes.rb | 1 + spec/requests/admin/llm_connections_spec.rb | 12 +++ spec/requests/admin/llm_models_spec.rb | 68 ++++++++++++++++ 13 files changed, 280 insertions(+), 71 deletions(-) create mode 100644 app/components/llm_connections/default_models_component.html.erb create mode 100644 app/components/llm_connections/default_models_component.rb create mode 100644 app/forms/llm_connections/default_models_form.rb diff --git a/app/components/llm_connections/default_models_component.html.erb b/app/components/llm_connections/default_models_component.html.erb new file mode 100644 index 000000000000..678eaf992c80 --- /dev/null +++ b/app/components/llm_connections/default_models_component.html.erb @@ -0,0 +1,39 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) the OpenProject GmbH + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License version 3. + +OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +Copyright (C) 2006-2013 Jean-Philippe Lang +Copyright (C) 2010-2013 the ChiliProject Team + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License +as published by the Free Software Foundation; either version 2 +of the License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +See COPYRIGHT and LICENSE files for more details. + +++#%> + +<%= + render(Primer::Beta::Subhead.new(mt: 4)) do |component| + component.with_heading(tag: :h3) { t("admin.llm_models.defaults.heading") } + component.with_description { t("admin.llm_models.defaults.description") } + end +%> + +<%= settings_primer_form_with(**form_options) do |f| %> + <%= render(LlmConnections::DefaultModelsForm.new(f)) %> +<% end %> diff --git a/app/components/llm_connections/default_models_component.rb b/app/components/llm_connections/default_models_component.rb new file mode 100644 index 000000000000..c417811b446b --- /dev/null +++ b/app/components/llm_connections/default_models_component.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module LlmConnections + # The "Default models" section of the LLMs page. + class DefaultModelsComponent < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + + alias_method :connection, :model + + private + + def form_options + { + model: connection, + url: url_helpers.defaults_llm_models_path, + method: :patch, + data: { test_selector: "llm-connection--defaults-form" } + } + end + end +end diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index d928bc6aa3bc..3bfdee45106b 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -38,8 +38,6 @@ class BaseContract < ModelContract attribute :api_key attribute :default_chat_model_id attribute :default_embedding_model_id - attribute :default_chat_model_id - attribute :default_embedding_model_id validates :base_url, presence: true validates :api_format, inclusion: { in: Llm::Adapters::FORMATS } @@ -59,18 +57,13 @@ class BaseContract < ModelContract private - # The mirror image of default_embedding_model_can_embed: a model the server - # positively identifies as an embedding model is not a chat candidate. + # A model the server identifies as an embedding model is not a chat candidate. def default_chat_model_can_chat llm_model = model.default_chat_model return if llm_model.blank? return unless model.changed_attributes.include?("default_chat_model_id") - embedding = model.capability_verdicts - .for_capability(:embeddings) - .exists?(model_id: llm_model.external_id, state: "supported") - - errors.add(:default_chat_model_id, :cannot_chat) if embedding + errors.add(:default_chat_model_id, :cannot_chat) if model.default_chat_model&.embedding? end def features_require_connection diff --git a/app/controllers/admin/llm_connections_controller.rb b/app/controllers/admin/llm_connections_controller.rb index 302033c60a30..50061aebde98 100644 --- a/app/controllers/admin/llm_connections_controller.rb +++ b/app/controllers/admin/llm_connections_controller.rb @@ -123,7 +123,7 @@ def redirect_with_error(message) # saved value, so submitting it unchanged posts an empty string. def llm_connection_params permitted = params.expect( - llm_connection: %i[llm_features_enabled api_format base_url api_key default_chat_model_id] + llm_connection: %i[llm_features_enabled api_format base_url api_key] ) permitted.delete(:api_key) if permitted[:api_key].blank? permitted.to_h.symbolize_keys diff --git a/app/controllers/admin/llm_models_controller.rb b/app/controllers/admin/llm_models_controller.rb index 825a4813288f..45e0cb0f6f25 100644 --- a/app/controllers/admin/llm_models_controller.rb +++ b/app/controllers/admin/llm_models_controller.rb @@ -124,6 +124,17 @@ def destroy redirect_to llm_models_path, status: :see_other end + def update_defaults + result = ::LlmConnections::UpdateService + .new(user: current_user, model: @connection) + .call(**default_model_params) + + result.on_success { flash[:notice] = t("admin.llm_models.defaults.success") } + result.on_failure { flash[:error] = result.errors.full_messages.join(", ") } + + redirect_to llm_models_path, status: :see_other + end + # Hides a model from the pickers, or puts it back. Deliberately does not # touch +active+, which the catalogue sync owns and would overwrite. def toggle @@ -140,6 +151,10 @@ def toggle private + def default_model_params + params.expect(llm_connection: %i[default_chat_model_id]).to_h.symbolize_keys + end + def set_connection @connection = LlmConnection.active_connection end diff --git a/app/forms/llm_connections/connection_form.rb b/app/forms/llm_connections/connection_form.rb index 2c0b33eaf05d..8669f08b9968 100644 --- a/app/forms/llm_connections/connection_form.rb +++ b/app/forms/llm_connections/connection_form.rb @@ -103,31 +103,6 @@ class ConnectionForm < ApplicationForm end end - if models_available? - # An autocompleter rather than a select: a gateway reports hundreds of - # models, and every one of them would otherwise be inlined as an option - # in the page body. decorated: true serialises the list into the element, - # so this needs no endpoint of its own. - f.autocompleter( - name: :default_chat_model_id, - label: LlmConnection.human_attribute_name(:default_chat_model_id), - caption: I18n.t("admin.llm_connections.form.default_chat_model_caption"), - autocomplete_options: { - decorated: true, - inputValue: model.default_chat_model_id, - placeholder: I18n.t("label_none_parentheses") - } - ) do |list| - list.option(label: I18n.t("label_none_parentheses"), value: "", - selected: model.default_chat_model_id.blank?) - - default_chat_model_options.each do |model_id| - list.option(label: option_label(model_id), value: model_id, - selected: model.default_chat_model_id == model_id) - end - end - end - f.submit( name: :submit, label: submit_label, @@ -167,41 +142,6 @@ def server_descriptions ) end - def models_available? - model.available_model_ids.any? - end - - # Deactivated models are hidden, and so is anything positively known to be - # an embedding model -- it is a different kind of model, not a chat choice. - # The one already chosen is kept regardless: dropping it would silently - # blank the field on the next save. - def default_chat_model_options - chat_capable = model.selectable_model_ids.reject { |id| embeddings_state(id) == :supported } - - (chat_capable + [model.default_chat_model_id]).compact_blank.uniq - end - - # The same friendly name the model table shows; the identifier stays the value. - def option_label(model_id) - model_names[model_id].presence || model_id - end - - def model_names - @model_names ||= model.models.pluck(:external_id, :display_name).to_h - end - - # No verdict at all is the same as an inconclusive one: we do not know. - def embeddings_state(model_id) - embeddings_verdicts[model_id]&.to_sym || :unknown - end - - def embeddings_verdicts - @embeddings_verdicts ||= model.capability_verdicts - .for_capability(:embeddings) - .pluck(:model_id, :state) - .to_h - end - def submit_label model.persisted? ? I18n.t(:button_save) : I18n.t("admin.llm_connections.form.button_connect") end diff --git a/app/forms/llm_connections/default_models_form.rb b/app/forms/llm_connections/default_models_form.rb new file mode 100644 index 000000000000..afb34a49d9f8 --- /dev/null +++ b/app/forms/llm_connections/default_models_form.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module LlmConnections + class DefaultModelsForm < ApplicationForm + form do |f| + # An autocompleter rather than a select: a gateway reports hundreds of + # models, and every one of them would otherwise be inlined as an option in + # the page body. decorated: true serialises the list into the element, so + # this needs no endpoint of its own. + f.autocompleter( + name: :default_chat_model_id, + label: LlmConnection.human_attribute_name(:default_chat_model_id), + caption: I18n.t("admin.llm_models.defaults.chat_caption"), + autocomplete_options: { + decorated: true, + inputValue: model.default_chat_model_id, + placeholder: I18n.t("label_none_parentheses") + } + ) do |list| + list.option(label: I18n.t("label_none_parentheses"), value: "", + selected: model.default_chat_model_id.blank?) + + default_chat_model_options.each do |model_id| + list.option(label: option_label(model_id), value: model_id, + selected: model.default_chat_model_id == model_id) + end + end + + f.submit(name: :submit, label: I18n.t(:button_save), scheme: :primary) + end + + private + + # The one already chosen is kept regardless of what the server offers today: + # dropping it would silently blank the field on the next save. + def default_chat_model_options + (model.chat_model_ids + [model.default_chat_model_id]).compact_blank.uniq + end + + # The same friendly name the model table shows; the identifier stays the value. + def option_label(model_id) + model_names[model_id].presence || model_id + end + + def model_names + @model_names ||= model.models.pluck(:external_id, :display_name).to_h + end + end +end diff --git a/app/models/llm_connection.rb b/app/models/llm_connection.rb index 90cc82742ed5..cfb26c7bb233 100644 --- a/app/models/llm_connection.rb +++ b/app/models/llm_connection.rb @@ -109,6 +109,14 @@ def selectable_model_ids models.selectable.by_identifier.pluck(:external_id) end + def embedding_model_ids + embedding = capability_verdicts.for_capability(:embeddings).where(state: "supported").pluck(:model_id) + + selectable_model_ids & embedding + end + + def chat_model_ids = selectable_model_ids - embedding_model_ids + def server_flavour options["server_flavour"].presence&.to_sym end diff --git a/app/views/admin/llm_models/index.html.erb b/app/views/admin/llm_models/index.html.erb index 23e6a4e445b2..7a3b59f0b0c6 100644 --- a/app/views/admin/llm_models/index.html.erb +++ b/app/views/admin/llm_models/index.html.erb @@ -61,5 +61,7 @@ See COPYRIGHT and LICENSE files for more details. %> <% end %> +<%= render(LlmConnections::DefaultModelsComponent.new(@connection)) %> + <%= render(LlmConnections::Models::SubHeaderComponent.new(@query)) %> <%= render(LlmConnections::Models::IndexComponent.new(@models, connection: @connection)) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 34624414ae52..42a20aaae861 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1737,7 +1737,6 @@ en: api_key_remove: "Remove key" base_url_caption: "The base URL of the server, for example https://example.com/v1." button_connect: "Connect" - default_chat_model_caption: "Used by AI features that do not select a model themselves." llm_features_enabled_caption: "When turned off, all LLM-backed AI features stop working. The connection settings are kept." label_connecting: "Contacting the LLM server…" server_description: "Connect OpenProject to a server that speaks the %{api_format} API, so AI features can use it. The host URL below decides which server that is." @@ -1755,6 +1754,11 @@ en: llm_models: create: success: "%{model} has been added." + defaults: + chat_caption: "Only models that are switched on in the list below are offered." + description: "Used by AI features that do not choose a model themselves." + heading: "Default models" + success: "The default models have been saved." destroy: description: "The model will no longer be offered to AI features." description_bound: "This model is currently used by %{features}. Those features will stop working until another model is selected." diff --git a/config/routes.rb b/config/routes.rb index 94da62f440ee..8b0db7bd165c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -753,6 +753,7 @@ collection do get :search, defaults: { format: :turbo_stream } post :refresh + patch :defaults, action: :update_defaults end member do diff --git a/spec/requests/admin/llm_connections_spec.rb b/spec/requests/admin/llm_connections_spec.rb index da2d57dd3a59..52b4c3088fb6 100644 --- a/spec/requests/admin/llm_connections_spec.rb +++ b/spec/requests/admin/llm_connections_spec.rb @@ -360,4 +360,16 @@ expect(connection.reload.api_key).to eq("sk-test") end end + + describe "the default models" do + let!(:connection) { create(:llm_connection, :with_models, base_url: "https://example.com/v1") } + + before { login_as admin } + + it "are chosen on the LLMs page, not here" do + patch llm_connection_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b" } } + + expect(connection.reload.default_chat_model_id).to be_nil + end + end end diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index e842731e3c8c..894bb89b458c 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -36,6 +36,14 @@ let(:admin) { create(:admin) } let(:base_url) { "https://example.com/v1" } + # The picker is an autocompleter, so its options are serialised into the + # element rather than rendered as markup. + def offered_default_models + items = page.find("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter")["data-items"] + + JSON.parse(items).pluck("id").compact_blank + end + describe "with the feature flag off", with_flag: { llm_connection: false } do before { login_as admin } @@ -125,6 +133,29 @@ expect(cell.find(".Label")[:title]).to eq("Reported by the server") end + it "offers the default chat model next to the models it may be chosen from" do + connection = create(:llm_connection, :with_models, base_url:) + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + + get llm_models_path + + expect(response.body).to include("Default models") + # An embedding model is a different kind of model, not a chat choice. + expect(offered_default_models).to contain_exactly("qwen3.6-27b") + end + + it "keeps a stored default listed once its model is switched off" do + connection = create(:llm_connection, :with_models, base_url:) + chat_model = connection.models.find_by(external_id: "qwen3.6-27b") + connection.update!(default_chat_model: chat_model) + chat_model.update!(deactivated_at: Time.current) + + get llm_models_path + + expect(offered_default_models).to include("qwen3.6-27b") + end + it "sends the administrator to the settings while the features are off", with_settings: { llm_features_enabled: false } do create(:llm_connection, :with_models, base_url:) @@ -585,6 +616,43 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size end end + describe "PATCH /admin/llm_models/defaults" do + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + + before { login_as admin } + + it "stores the default chat model without contacting the server" do + patch defaults_llm_models_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b" } } + + expect(response).to redirect_to(llm_models_path) + expect(connection.reload.default_chat_model_id).to eq("qwen3.6-27b") + expect(flash[:notice]).to eq("The default models have been saved.") + expect(a_request(:get, "#{base_url}/models")).not_to have_been_made + end + + it "refuses a model the server does not offer" do + patch defaults_llm_models_path, params: { llm_connection: { default_chat_model_id: "not-there" } } + + expect(connection.reload.default_chat_model_id).to be_nil + expect(flash[:error]).to be_present + end + + it "leaves the server settings alone" do + patch defaults_llm_models_path, + params: { llm_connection: { default_chat_model_id: "qwen3.6-27b", base_url: "https://elsewhere.test/v1" } } + + expect(connection.reload.base_url).to eq(base_url) + end + + it "is refused to a non-admin" do + login_as create(:user) + + patch defaults_llm_models_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b" } } + + expect(connection.reload.default_chat_model_id).to be_nil + end + end + describe "POST /admin/llm_models/:id/toggle" do let!(:connection) { create(:llm_connection, :enabled, base_url:) } let!(:llm_model) { create(:llm_model, llm_connection: connection, external_id: "qwen3.6-27b") } From 61abe882c37f8967aab7f6dd868e49c62c135aee Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Fri, 4 Sep 2026 16:09:59 +0200 Subject: [PATCH 4/8] [AI-3] Turn the deactivation spec into a generic model spec spec/models/llm_model_deactivation_spec.rb described one scenario rather than the model, which is not how model specs are written here. Its examples now live in a "deactivation" block of spec/models/llm_model_spec.rb. The half sentence left behind in that file claimed the invariant of this branch, that switching a model off never breaks a feature already pointing at it, without testing it. Two examples now cover it: the model stays in available_model_ids, and the contract still accepts a default naming a model an administrator switched off. --- .../llm_connections/update_contract_spec.rb | 11 +++ spec/models/llm_model_deactivation_spec.rb | 67 ------------------- spec/models/llm_model_spec.rb | 37 ++++++++++ 3 files changed, 48 insertions(+), 67 deletions(-) delete mode 100644 spec/models/llm_model_deactivation_spec.rb diff --git a/spec/contracts/llm_connections/update_contract_spec.rb b/spec/contracts/llm_connections/update_contract_spec.rb index 8a8225745530..0927363a402e 100644 --- a/spec/contracts/llm_connections/update_contract_spec.rb +++ b/spec/contracts/llm_connections/update_contract_spec.rb @@ -226,5 +226,16 @@ include_examples "contract is invalid", default_chat_model_id: :cannot_chat end + + # Curation, not enforcement: switching a model off hides it from the pickers + # and must never break a feature that already points at it. + context "with a model an administrator switched off" do + before do + connection.models.find_by(external_id: "qwen3.6-27b").update!(deactivated_at: Time.current) + connection.default_chat_model_id = "qwen3.6-27b" + end + + include_examples "contract is valid" + end end end diff --git a/spec/models/llm_model_deactivation_spec.rb b/spec/models/llm_model_deactivation_spec.rb deleted file mode 100644 index e75e259c6901..000000000000 --- a/spec/models/llm_model_deactivation_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -# frozen_string_literal: true - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) the OpenProject GmbH -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License version 3. -# -# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -# -# See COPYRIGHT and LICENSE files for more details. -#++ - -require "spec_helper" - -RSpec.describe LlmModel, "deactivation", :llm_server_helpers, :webmock, - with_flag: { llm_connection: true } do - let(:base_url) { "https://example.com/v1" } - let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } - let(:model) { connection.models.find_by(external_id: "qwen3.6-27b") } - - before { model.update!(deactivated_at: Time.current) } - - it "hides the model from the pickers" do - expect(connection.selectable_model_ids).not_to include("qwen3.6-27b") - expect(connection.selectable_model_ids).to include("bge-m3") - end - - # administrator switches off must never silently break a running feature. - - # The reason deactivated_at exists rather than reusing active: the sync writes - # active on every refresh, so an administrator's choice stored there would be - # undone by the next "Refresh models". - it "survives a catalogue sync that still reports the model" do - mock_llm_models_response(base_url) - - LlmConnections::SyncModelsService.new(connection).call - - expect(model.reload).to be_deactivated - expect(model).to be_active - expect(model).not_to be_selectable - end - - it "is distinct from a model the server withdrew" do - withdrawn = create(:llm_model, :withdrawn, llm_connection: connection, external_id: "gone") - - expect(withdrawn).to be_withdrawn - expect(model).not_to be_withdrawn - end -end diff --git a/spec/models/llm_model_spec.rb b/spec/models/llm_model_spec.rb index b9143bef6ace..6fc20f414589 100644 --- a/spec/models/llm_model_spec.rb +++ b/spec/models/llm_model_spec.rb @@ -55,4 +55,41 @@ expect(llm_model.model_type).to eq(:chat) end end + + describe "deactivation", :llm_server_helpers, :webmock, with_flag: { llm_connection: true } do + let(:base_url) { "https://example.com/v1" } + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let(:model) { connection.models.find_by(external_id: "qwen3.6-27b") } + + before { model.update!(deactivated_at: Time.current) } + + it "hides the model from the pickers" do + expect(connection.selectable_model_ids).not_to include("qwen3.6-27b") + expect(connection.selectable_model_ids).to include("bge-m3") + end + + it "stays addressable for a feature that is already bound to it" do + expect(connection.available_model_ids).to include("qwen3.6-27b") + end + + # The reason deactivated_at exists rather than reusing active: the sync writes + # active on every refresh, so an administrator's choice stored there would be + # undone by the next "Refresh models". + it "survives a catalogue sync that still reports the model" do + mock_llm_models_response(base_url) + + LlmConnections::SyncModelsService.new(connection).call + + expect(model.reload).to be_deactivated + expect(model).to be_active + expect(model).not_to be_selectable + end + + it "is distinct from a model the server withdrew" do + withdrawn = create(:llm_model, :withdrawn, llm_connection: connection, external_id: "gone") + + expect(withdrawn).to be_withdrawn + expect(model).not_to be_withdrawn + end + end end From ef82606941baec6b40cfc7d1f5ee7af0cc8ccf5a Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Fri, 4 Sep 2026 16:47:44 +0200 Subject: [PATCH 5/8] [AI-3] Ask for a default only once models are stored The picker used to be skipped on the settings page while the catalogue was empty. Moving it to the AI models page dropped that guard, so a connection whose server has reported nothing showed a Default models section offering only "(none)" right above the empty model table. The section is left out until there is something to choose from. --- .../llm_connections/default_models_component.rb | 3 +++ spec/requests/admin/llm_models_spec.rb | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/app/components/llm_connections/default_models_component.rb b/app/components/llm_connections/default_models_component.rb index c417811b446b..4cbb4a3476c9 100644 --- a/app/components/llm_connections/default_models_component.rb +++ b/app/components/llm_connections/default_models_component.rb @@ -36,6 +36,9 @@ class DefaultModelsComponent < ApplicationComponent alias_method :connection, :model + # Nothing to choose from, and the empty table right below says so. + def render? = connection.available_model_ids.any? + private def form_options diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 894bb89b458c..8cd8488fbf5f 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -145,6 +145,15 @@ def offered_default_models expect(offered_default_models).to contain_exactly("qwen3.6-27b") end + it "asks for no default while the connection has no model to offer" do + create(:llm_connection, :enabled, base_url:) + + get llm_models_path + + expect(response.body).to include("No models available") + expect(response.body).not_to include("Default models") + end + it "keeps a stored default listed once its model is switched off" do connection = create(:llm_connection, :with_models, base_url:) chat_model = connection.models.find_by(external_id: "qwen3.6-27b") From 7c0b752121b59f74106b4803503f323f5ca80264 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Fri, 4 Sep 2026 17:23:43 +0200 Subject: [PATCH 6/8] [AI-3] Refresh the default pickers when a model is toggled The Status toggle answered with JSON, so the Default models section next to the table kept offering a model that had just been switched off, and kept withholding one that had just been switched on, until the page was loaded again. The toggle now asks for a turbo stream and the action re-renders LlmConnections::DefaultModelsComponent, which is wrapped for that purpose. Reported in the review of the follow-up commits (finding 5). --- .../default_models_component.html.erb | 20 ++++++++++--------- .../default_models_component.rb | 1 + .../llm_connections/models_row_component.rb | 2 +- .../admin/llm_models_controller.rb | 5 ++++- config/routes.rb | 2 +- spec/features/admin/llm_connection_spec.rb | 14 +++++++++++-- spec/requests/admin/llm_models_spec.rb | 16 +++++++++++++-- 7 files changed, 44 insertions(+), 16 deletions(-) diff --git a/app/components/llm_connections/default_models_component.html.erb b/app/components/llm_connections/default_models_component.html.erb index 678eaf992c80..eb268c6c1fb4 100644 --- a/app/components/llm_connections/default_models_component.html.erb +++ b/app/components/llm_connections/default_models_component.html.erb @@ -27,13 +27,15 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= - render(Primer::Beta::Subhead.new(mt: 4)) do |component| - component.with_heading(tag: :h3) { t("admin.llm_models.defaults.heading") } - component.with_description { t("admin.llm_models.defaults.description") } - end -%> - -<%= settings_primer_form_with(**form_options) do |f| %> - <%= render(LlmConnections::DefaultModelsForm.new(f)) %> +<%= component_wrapper do %> + <%= + render(Primer::Beta::Subhead.new(mt: 4)) do |component| + component.with_heading(tag: :h3) { t("admin.llm_models.defaults.heading") } + component.with_description { t("admin.llm_models.defaults.description") } + end + %> + + <%= settings_primer_form_with(**form_options) do |f| %> + <%= render(LlmConnections::DefaultModelsForm.new(f)) %> + <% end %> <% end %> diff --git a/app/components/llm_connections/default_models_component.rb b/app/components/llm_connections/default_models_component.rb index 4cbb4a3476c9..33762c03cf74 100644 --- a/app/components/llm_connections/default_models_component.rb +++ b/app/components/llm_connections/default_models_component.rb @@ -33,6 +33,7 @@ module LlmConnections class DefaultModelsComponent < ApplicationComponent include ApplicationHelper include OpPrimer::ComponentHelpers + include OpTurbo::Streamable alias_method :connection, :model diff --git a/app/components/llm_connections/models_row_component.rb b/app/components/llm_connections/models_row_component.rb index edaf9a633bd9..2d1fc7f5f9ac 100644 --- a/app/components/llm_connections/models_row_component.rb +++ b/app/components/llm_connections/models_row_component.rb @@ -106,7 +106,7 @@ def mutation_options { src: url_helpers.toggle_llm_model_path(llm_model), csrf_token: helpers.form_authenticity_token, - data: { "turbo-method": :post, "turbo-stream": true }, + turbo: true, classes: "op-primer-adjustments__toggle-switch--hidden-loading-indicator" } end diff --git a/app/controllers/admin/llm_models_controller.rb b/app/controllers/admin/llm_models_controller.rb index 45e0cb0f6f25..39e813fd97ad 100644 --- a/app/controllers/admin/llm_models_controller.rb +++ b/app/controllers/admin/llm_models_controller.rb @@ -146,7 +146,10 @@ def toggle llm_model.update!(deactivated_at: llm_model.deactivated? ? nil : Time.current) - render json: {}, status: :ok + # The default pickers offer the models that are switched on, so they go + # stale the moment a toggle flips. + update_via_turbo_stream(component: ::LlmConnections::DefaultModelsComponent.new(@connection)) + respond_with_turbo_streams end private diff --git a/config/routes.rb b/config/routes.rb index 8b0db7bd165c..4d0fab08987e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -758,7 +758,7 @@ member do get :delete_dialog - post :toggle + post :toggle, defaults: { format: :turbo_stream } end end diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index 1b8d947ecf73..20ebc087c244 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -49,6 +49,12 @@ # The kebab is a Primer ActionMenu: clicking it before its behaviour is # attached silently does nothing, so wait for the page to settle first and # for the item itself to become visible. + def offered_default_models + items = find("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter")["data-items"] + + JSON.parse(items).pluck("id").compact_blank + end + def choose_action(item) expect(page).to have_test_selector("llm-connection--actions") find_test_selector("llm-connection--actions").click @@ -150,12 +156,16 @@ def choose_action(item) llm_model = connection.models.find_by(external_id: "qwen3.6-27b") visit llm_models_path + + expect(offered_default_models).to include("qwen3.6-27b") + find_test_selector("llm-model--toggle-#{llm_model.id}").click wait_for { llm_model.reload.deactivated_at }.not_to be_nil + wait_for { offered_default_models }.not_to include("qwen3.6-27b") - # The toggle acknowledges with JSON rather than re-rendering the row, so - # the table only catches up on the next load. + # The toggle re-renders the pickers, not the row, so the table itself only + # catches up on the next load. visit llm_models_path within_test_selector("llm-model--toggle-#{llm_model.id}") do diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 8cd8488fbf5f..c897d7ccc3ab 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -38,12 +38,16 @@ # The picker is an autocompleter, so its options are serialised into the # element rather than rendered as markup. - def offered_default_models - items = page.find("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter")["data-items"] + def offered_default_models(markup = page) + items = markup.find("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter")["data-items"] JSON.parse(items).pluck("id").compact_blank end + # Nokogiri does not descend into a