From 0a6edafd110672804160e27d13c7ceeecaf37284 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Sat, 22 Aug 2026 18:12:59 +0200 Subject: [PATCH 01/10] [AI-3] Register AI features and resolve their models at runtime Adds the feature registry: each AI feature declares its kind, the capabilities it requires and whether administrators may override its model, following the FeatureDecisions idiom and living in lib_static for the same reload-safety reason. The AI models page then lets an administrator bind each registered feature to a model, fed by a query that never hides a model: an option is selectable, selectable with a warning when a required capability is unknown, or disabled with the reason when it is ruled out. Llm::Runtime is the single place a feature's model is resolved: explicit override, then binding, then the connection default, and otherwise unbound. It fails closed with a machine-readable reason rather than substituting another model, because a text transform with a different model is a different feature. Bindings reference models by identifier string, never by foreign key, so a binding survives its model disappearing and the dangling state is derived, not stored. With something to bind, the connection form gains the default embedding model selector, offering only models actually known to embed, and the destructive dialogs now name the features that would be affected. Part 10 of the AI-3 stack. https://community.openproject.org/work_packages/66020 --- .../delete_model_dialog_component.rb | 14 +- .../disconnect_dialog_component.html.erb | 8 +- .../disconnect_dialog_component.rb | 4 + .../feature_binding_component.html.erb | 53 +++++ .../feature_binding_component.rb | 111 +++++++++++ .../llm_connections/base_contract.rb | 17 ++ .../admin/llm_feature_bindings_controller.rb | 129 ++++++++++++ .../llm_connections/feature_binding_form.rb | 143 ++++++++++++++ app/models/llm_connection.rb | 1 + app/models/llm_feature_binding.rb | 135 +++++++++++++ app/models/llm_model.rb | 1 + app/services/llm/runtime.rb | 162 +++++++++++++++ .../selectable_models_query.rb | 96 +++++++++ .../admin/llm_feature_bindings/index.html.erb | 63 ++++++ config/initializers/llm_features.rb | 52 +++++ config/initializers/menus.rb | 6 + config/locales/en.yml | 53 +++++ config/routes.rb | 4 + ...60811140100_create_llm_feature_bindings.rb | 52 +++++ lib_static/open_project/llm/features.rb | 120 +++++++++++ spec/features/admin/llm_connection_spec.rb | 14 ++ spec/models/llm_model_spec.rb | 15 ++ spec/requests/admin/llm_connections_spec.rb | 6 + .../admin/llm_feature_bindings_spec.rb | 186 ++++++++++++++++++ spec/requests/admin/llm_models_spec.rb | 64 ++++++ spec/services/llm/runtime_spec.rb | 176 +++++++++++++++++ 26 files changed, 1680 insertions(+), 5 deletions(-) create mode 100644 app/components/llm_connections/feature_binding_component.html.erb create mode 100644 app/components/llm_connections/feature_binding_component.rb create mode 100644 app/controllers/admin/llm_feature_bindings_controller.rb create mode 100644 app/forms/llm_connections/feature_binding_form.rb create mode 100644 app/models/llm_feature_binding.rb create mode 100644 app/services/llm/runtime.rb create mode 100644 app/services/llm_connections/selectable_models_query.rb create mode 100644 app/views/admin/llm_feature_bindings/index.html.erb create mode 100644 config/initializers/llm_features.rb create mode 100644 db/migrate/20260811140100_create_llm_feature_bindings.rb create mode 100644 lib_static/open_project/llm/features.rb create mode 100644 spec/requests/admin/llm_feature_bindings_spec.rb create mode 100644 spec/services/llm/runtime_spec.rb diff --git a/app/components/llm_connections/delete_model_dialog_component.rb b/app/components/llm_connections/delete_model_dialog_component.rb index e4c7b82bfee6..7cdb5941d7bf 100644 --- a/app/components/llm_connections/delete_model_dialog_component.rb +++ b/app/components/llm_connections/delete_model_dialog_component.rb @@ -41,11 +41,17 @@ def form_arguments { action: url_helpers.llm_model_path(llm_model), method: :delete } end - # Named so the message says what is actually at stake. The connection - # defaults count as bindings here -- deleting their model breaks every - # feature that inherits them. + # Named so the message says what is actually at stake: features bound to this + # model stop resolving, rather than silently falling back to another one. + # The connection defaults count as bindings here -- deleting their model + # breaks every feature that inherits them. def bound_features - affected_defaults + bindings = llm_model.llm_connection + .feature_bindings + .where(model_id: llm_model.external_id) + .filter_map { |binding| binding.feature&.label } + + bindings + affected_defaults end def affected_defaults diff --git a/app/components/llm_connections/disconnect_dialog_component.html.erb b/app/components/llm_connections/disconnect_dialog_component.html.erb index bbcda28dc1fa..721e2ac22975 100644 --- a/app/components/llm_connections/disconnect_dialog_component.html.erb +++ b/app/components/llm_connections/disconnect_dialog_component.html.erb @@ -21,7 +21,13 @@ safe_join( [ content_tag(:li, t("admin.llm_connections.disconnect.keeps_settings")), - content_tag(:li, t("admin.llm_connections.disconnect.keeps_models")) + content_tag(:li, t("admin.llm_connections.disconnect.keeps_models")), + if bound_features.any? + content_tag( + :li, + t("admin.llm_connections.disconnect.keeps_bindings", features: bound_features.to_sentence) + ) + end ].compact ) end diff --git a/app/components/llm_connections/disconnect_dialog_component.rb b/app/components/llm_connections/disconnect_dialog_component.rb index eb7413199790..785e8aa28b69 100644 --- a/app/components/llm_connections/disconnect_dialog_component.rb +++ b/app/components/llm_connections/disconnect_dialog_component.rb @@ -46,5 +46,9 @@ class DisconnectDialogComponent < ApplicationComponent def form_arguments { action: url_helpers.disconnect_llm_connection_path, method: :post } end + + def bound_features + connection.feature_bindings.filter_map { |binding| binding.feature&.label if binding.model_id.present? } + end end end diff --git a/app/components/llm_connections/feature_binding_component.html.erb b/app/components/llm_connections/feature_binding_component.html.erb new file mode 100644 index 000000000000..762e4a7b5705 --- /dev/null +++ b/app/components/llm_connections/feature_binding_component.html.erb @@ -0,0 +1,53 @@ +<%= render(Primer::Box.new(border: true, border_radius: 2, p: 3, mb: 3)) do %> + <%= render(Primer::Beta::Text.new(tag: :h3, font_size: 4, font_weight: :bold, mb: 1)) { feature.label } %> + + <% if feature.caption.present? %> + <%= render(Primer::Beta::Text.new(tag: :p, color: :muted, mb: 2)) { feature.caption } %> + <% end %> + + <% if dangling? %> + <%= render(Primer::Alpha::Banner.new(scheme: :warning, mb: 2, icon: :alert)) do %> + <%= t("admin.llm_feature_bindings.dangling", model: binding.resolved_model_id) %> + <% end %> + <% end %> + + <% if deactivated? %> + <%= render(Primer::Alpha::Banner.new(scheme: :warning, mb: 2, icon: :alert)) do %> + <%= t("admin.llm_feature_bindings.deactivated", model: binding.resolved_model_id) %> + <% end %> + <% end %> + + <% if locked? %> + <%= render(Primer::Alpha::Banner.new(scheme: :warning, mb: 2, icon: :lock)) do %> + <%= t("admin.llm_feature_bindings.locked", model: binding.model_id) %> + <% end %> + <% end %> + + <%= primer_form_with(model: form_model, url: form_url, method: :patch, scope: :llm_feature_binding) do |f| %> + <%= render( + LlmConnections::FeatureBindingForm.new( + f, + options: model_options, + inherit_label:, + feature_key: feature.key, + locked: locked?, + embedding: feature.embedding?, + selected_model_id: binding&.model_id, + dimensions_hint: probed_dimensions + ) + ) %> + <% end %> + + <% if locked? && feature.embedding? %> + <%# Rendered as text rather than disabled inputs: a disabled input submits + nothing, so the values would arrive blank and wipe the columns. %> + <%= render(Primer::Beta::Text.new(tag: :p, font_weight: :bold, mt: 2, mb: 1)) do %> + <%= t("admin.llm_feature_bindings.locked_values_heading") %> + <% end %> + <% locked_values.each do |label, value| %> + <%= render(Primer::Beta::Text.new(tag: :p, color: :muted, mb: 0)) do %> + <%= "#{label}: #{value}" %> + <% end %> + <% end %> + <% end %> +<% end %> diff --git a/app/components/llm_connections/feature_binding_component.rb b/app/components/llm_connections/feature_binding_component.rb new file mode 100644 index 000000000000..9d5d16e5a555 --- /dev/null +++ b/app/components/llm_connections/feature_binding_component.rb @@ -0,0 +1,111 @@ +# 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 + # One feature's row on the model assignment page. + class FeatureBindingComponent < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + + def initialize(feature:, connection:, binding: nil) + super(feature) + @feature = feature + @connection = connection + @binding = binding + end + + # The record the select binds to. A feature without a stored binding still + # needs one so the form has a model_id to read. + def form_model + binding || connection.feature_bindings.new(feature_key: feature.key.to_s) + end + + # Not named +options+: ApplicationComponent already owns that name and + # initialises it to an empty hash, which silently swallowed the memoisation. + def model_options + @model_options ||= SelectableModelsQuery.new(connection, feature).call + end + + def inherit_label + if default_model_id.present? + I18n.t("admin.llm_feature_bindings.inherit_with_default", model: default_model_id) + else + I18n.t("admin.llm_feature_bindings.inherit_without_default") + end + end + + def locked? = binding&.locked? + + def dangling? = binding&.dangling? + + # What the embeddings probe last saw, offered as information. Never filled + # into the field: the server decides the vector size at index time. + def probed_dimensions + return unless feature.embedding? + + model_id = binding&.resolved_model_id + return if model_id.blank? + + connection.capability_verdicts.for_model(model_id).for_capability(:embeddings).first&.dimensions + end + + # Quoted so a trailing space -- load-bearing for the E5 and BGE families -- + # is visible rather than invisible. + def locked_values + [ + [LlmFeatureBinding.human_attribute_name(:model_id), binding.model_id], + [LlmFeatureBinding.human_attribute_name(:dimensions), binding.dimensions || "—"], + [LlmFeatureBinding.human_attribute_name(:input_prefix), binding.input_prefix.to_s.inspect], + [LlmFeatureBinding.human_attribute_name(:query_prefix), binding.query_prefix.to_s.inspect] + ] + end + + # Still resolvable, so not dangling -- but an administrator has hidden it + # from the pickers, so say so rather than let the choice look unremarkable. + def deactivated? + model_id = binding&.resolved_model_id + return false if model_id.blank? + + connection.models.deactivated.exists?(external_id: model_id) + end + + private + + attr_reader :feature, :connection, :binding + + def default_model_id + feature.embedding? ? connection.default_embedding_model_id : connection.default_chat_model_id + end + + def form_url + url_helpers.llm_feature_binding_path(feature.key) + end + end +end diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index 9865a6b8eb39..62aac5aec963 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -54,6 +54,7 @@ class BaseContract < ModelContract validate :features_require_connection validate :default_models_offered_by_server validate :default_chat_model_can_chat + validate :default_embedding_model_can_embed validate :not_configured_from_env def not_configured_from_env @@ -64,6 +65,22 @@ def not_configured_from_env private + # A model the server has positively told us cannot embed is not a candidate + # for the embedding default, however it got submitted. An unknown verdict + # does not block: that is the normal state for a server that publishes + # nothing about its models. + def default_embedding_model_can_embed + model_id = model.default_embedding_model_id + return if model_id.blank? + return unless model.changed_attributes.include?("default_embedding_model_id") + + unsupported = model.capability_verdicts + .for_capability(:embeddings) + .exists?(model_id:, state: "unsupported") + + errors.add(:default_embedding_model_id, :cannot_embed) if unsupported + end + # 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 diff --git a/app/controllers/admin/llm_feature_bindings_controller.rb b/app/controllers/admin/llm_feature_bindings_controller.rb new file mode 100644 index 000000000000..c531695799ee --- /dev/null +++ b/app/controllers/admin/llm_feature_bindings_controller.rb @@ -0,0 +1,129 @@ +# 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 Admin + # Assigns a model to each registered AI feature. + class LlmFeatureBindingsController < ApplicationController + layout "admin" + menu_item :llm_feature_bindings + + before_action :require_feature + before_action :require_admin + before_action :set_connection + + def index + @features = OpenProject::Llm::Features.available + @bindings = bindings_by_feature_key + end + + def update + feature = OpenProject::Llm::Features[params[:id]] + assign(feature) + + redirect_to llm_feature_bindings_path, status: :see_other + rescue OpenProject::Llm::UnknownFeature + render_404 + end + + private + + def set_connection + @connection = LlmConnection.instance + end + + # The flag gates the endpoints, not only the menu entry: an unfinished page + # must not accept writes just because somebody knows the URL. + def require_feature + render_404 unless OpenProject::FeatureDecisions.llm_connection_active? + end + + def bindings_by_feature_key + @connection.feature_bindings.index_by(&:feature_key) + end + + def binding_for(feature) + @connection.feature_bindings.find_or_initialize_by(feature_key: feature.key.to_s) + end + + def assign(feature) + binding = build_binding(feature) + + unless binding.save + flash[:error] = binding.errors.full_messages.join(", ") + return + end + + confirm_assignment(feature, probe_capabilities(feature, binding)) + end + + # The probe may just have proven the chosen model cannot do what the feature + # requires; confirming that save would report a working configuration that + # Llm::Runtime immediately resolves as incapable. + def confirm_assignment(feature, verdict) + if verdict&.blocking? + flash[:error] = t("admin.llm_feature_bindings.update.model_incapable", + feature: feature.label, + capability: Llm::Capabilities.label(:embeddings)) + else + flash[:notice] = t("admin.llm_feature_bindings.update.success", feature: feature.label) + end + end + + def build_binding(feature) + binding = binding_for(feature) + binding.model_id = params.dig(:llm_feature_binding, :model_id).presence + + # Only ever accepted for the kind of feature they describe; the model + # rejects them elsewhere, and they are not read at all for a chat feature. + assign_embedding_settings(binding) if feature.embedding? + + binding + end + + # The prefixes are stored exactly as typed. The trailing space in "passage: " + # is load-bearing for the E5 and BGE families, so stripping would silently + # degrade retrieval. + def assign_embedding_settings(binding) + settings = params.fetch(:llm_feature_binding, {}) + + binding.dimensions = settings[:dimensions].presence + binding.input_prefix = settings[:input_prefix] + binding.query_prefix = settings[:query_prefix] + end + + # The verdict that actually matters is the one for the model an administrator + # just chose, so it is fetched now rather than left unknown until first use. + def probe_capabilities(feature, binding) + return if feature.requires.empty? || binding.model_id.blank? + + LlmConnections::DetectCapabilitiesService.new(@connection).detect(binding.model_id).result + end + end +end diff --git a/app/forms/llm_connections/feature_binding_form.rb b/app/forms/llm_connections/feature_binding_form.rb new file mode 100644 index 000000000000..cf7a50fbd5f7 --- /dev/null +++ b/app/forms/llm_connections/feature_binding_form.rb @@ -0,0 +1,143 @@ +# 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 model select for one registered feature. + class FeatureBindingForm < ApplicationForm + # Primer::Forms::Base.new assigns the builder itself and calls this with the + # remaining keywords, so the builder must not appear in the signature. + def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: false, dimensions_hint: nil, + selected_model_id: nil) + super() + @selected_model_id = selected_model_id + @model_options = options + @inherit_label = inherit_label + @feature_key = feature_key + @locked = locked + @embedding = embedding + @dimensions_hint = dimensions_hint + end + + form do |f| + # An autocompleter rather than a select, so a model can be found by typing + # among the hundreds a gateway reports. decorated: true serialises the list + # into the element, so no endpoint is needed. + f.autocompleter( + name: :model_id, + label: LlmFeatureBinding.human_attribute_name(:model_id), + disabled: locked, + autocomplete_options: { + decorated: true, + inputValue: selected_model_id, + placeholder: inherit_label + }, + data: { test_selector: "llm-feature-binding--model-#{feature_key}" } + ) do |list| + list.option(label: inherit_label, value: "", selected: selected_model_id.blank?) + + model_options.each do |option| + # Listed but not choosable when a required capability is known to be + # missing: hiding it would leave the reason invisible too. + list.option(label: option_label(option), + value: option.model_id, + selected: selected_model_id == option.model_id, + disabled: !option.selectable?) + end + end + + # Only for an embedding feature, and only while unlocked. A locked binding + # renders these as text instead: a disabled input submits nothing, so the + # values would arrive blank and wipe the columns. + if embedding && !locked + f.text_field( + name: :dimensions, + type: :number, + min: 1, + label: LlmFeatureBinding.human_attribute_name(:dimensions), + caption: dimensions_caption, + input_width: :small, + data: { test_selector: "llm-feature-binding--dimensions-#{feature_key}" } + ) + + f.text_field( + name: :input_prefix, + label: LlmFeatureBinding.human_attribute_name(:input_prefix), + caption: I18n.t("admin.llm_feature_bindings.form.input_prefix_caption"), + input_width: :medium, + data: { test_selector: "llm-feature-binding--input-prefix-#{feature_key}" } + ) + + f.text_field( + name: :query_prefix, + label: LlmFeatureBinding.human_attribute_name(:query_prefix), + caption: I18n.t("admin.llm_feature_bindings.form.query_prefix_caption"), + input_width: :medium, + data: { test_selector: "llm-feature-binding--query-prefix-#{feature_key}" } + ) + end + + unless locked + f.submit( + name: :submit, + label: I18n.t(:button_save), + scheme: :secondary, + data: { test_selector: "llm-feature-binding--submit-#{feature_key}" } + ) + end + end + + private + + attr_reader :model_options, :inherit_label, :feature_key, :locked, :embedding, :dimensions_hint, + :selected_model_id + + # Blank is the right default: the server decides the vector size, and baking + # in a number it may contradict helps nobody. Where the probe has already + # seen a vector, its size is offered as information rather than filled in. + def dimensions_caption + return I18n.t("admin.llm_feature_bindings.form.dimensions_caption") if dimensions_hint.blank? + + I18n.t("admin.llm_feature_bindings.form.dimensions_caption_probed", dimensions: dimensions_hint) + end + + def option_label(option) + case option.state + when :unsupported + I18n.t("admin.llm_feature_bindings.option_unsupported", + model: option.model_id, + capability: option.reasons.map { |reason| Llm::Capabilities.label(reason) }.join(", ")) + when :unknown + I18n.t("admin.llm_feature_bindings.option_unknown", model: option.model_id) + else + option.model_id + end + end + end +end diff --git a/app/models/llm_connection.rb b/app/models/llm_connection.rb index 37e5d0c7a0f3..e6582c6934fd 100644 --- a/app/models/llm_connection.rb +++ b/app/models/llm_connection.rb @@ -52,6 +52,7 @@ class LlmConnection < ApplicationRecord belongs_to :default_chat_model, class_name: "LlmModel", optional: true belongs_to :default_embedding_model, class_name: "LlmModel", optional: true has_many :capability_verdicts, class_name: "LlmCapabilityVerdict", dependent: :delete_all + has_many :feature_bindings, class_name: "LlmFeatureBinding", dependent: :delete_all validates :base_url, presence: true validate :single_active_connection, if: :active? diff --git a/app/models/llm_feature_binding.rb b/app/models/llm_feature_binding.rb new file mode 100644 index 000000000000..321b30ad6106 --- /dev/null +++ b/app/models/llm_feature_binding.rb @@ -0,0 +1,135 @@ +# 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. +#++ + +# Which model a registered feature uses. +# +# One row per feature, not per binding: rows are reconciled against the registry +# and are never destroyed when a feature deregisters, so flipping a feature flag +# does not lose the administrator's choice. +class LlmFeatureBinding < ApplicationRecord + belongs_to :llm_connection + + # Settings that describe how vectors are written, and so only mean anything + # for an embedding feature. + EMBEDDING_SETTINGS = %i[dimensions input_prefix query_prefix].freeze + + # Everything a stored index depends on. Changing any of it invalidates the + # vectors already written, not just the model. + LOCKED_SETTINGS = ([:model_id] + EMBEDDING_SETTINGS).freeze + + before_save :freeze_inherited_model_on_lock + + validates :feature_key, presence: true, uniqueness: { scope: :llm_connection_id } + validates :dimensions, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true + validate :feature_registered + validate :embedding_settings_only_for_embedding_features + validate :locked_settings_unchanged + + def feature + OpenProject::Llm::Features[feature_key] + rescue OpenProject::Llm::UnknownFeature + nil + end + + # NULL means "use the connection default for this kind of model". + def resolved_model_id + model_id.presence || default_model_id + end + + def inherits_default? = model_id.blank? + + # Derived, never stored. A status column would be a cache with no invalidation + # trigger, and would be stale exactly when it matters -- right after the remote + # catalogue changed. + def dangling? + resolved = resolved_model_id + resolved.present? && llm_connection.available_model_ids.exclude?(resolved) + end + + def locked? = locked_at.present? + + private + + # A lock freezes what the vectors were written with. A binding that inherits + # the connection default would keep following it after the lock, so the + # resolved model is written down at the moment of locking. + def freeze_inherited_model_on_lock + return unless locked_at.present? && locked_at_changed? && locked_at_was.nil? + return if model_id.present? + + self.model_id = default_model_id + end + + def default_model_id + return if feature.nil? + + feature.embedding? ? llm_connection.default_embedding_model_id : llm_connection.default_chat_model_id + end + + def feature_registered + return if feature.present? + + errors.add(:feature_key, :not_registered) + end + + def embedding_settings_only_for_embedding_features + return if feature.nil? || feature.embedding? + + EMBEDDING_SETTINGS.each do |attribute| + next if public_send(attribute).blank? + + errors.add(attribute, :not_for_chat_feature) + end + end + + # Vectors written under one embedding model are meaningless under another, and + # the dimension count is baked into the index, so a locked binding can only be + # changed by an explicit re-index. + # + # The prefixes are locked for the same reason and matter just as much: an index + # built with "passage: " but queried under a different prefix does not error, + # it quietly returns worse results, which is the hardest kind of failure to + # notice. + # + # TODO(#69620): re-indexing is what clears locked_at. Until that job exists a + # locked binding can only be changed in the database. + def locked_settings_unchanged + # Only constrains later edits. On the save that records the lock -- and on + # create -- every attribute reads as changed from nil, and there is nothing + # indexed yet for them to contradict. + return unless locked? && locked_at_was.present? + + LOCKED_SETTINGS.each do |attribute| + next unless public_send(:"#{attribute}_changed?") + + errors.add(attribute, :locked) + end + end +end diff --git a/app/models/llm_model.rb b/app/models/llm_model.rb index 75d9ac73b05f..c18de5fa99d2 100644 --- a/app/models/llm_model.rb +++ b/app/models/llm_model.rb @@ -65,6 +65,7 @@ def cascade_rename!(previous_external_id) return if previous_external_id.blank? || previous_external_id == external_id llm_connection.capability_verdicts.where(model_id: previous_external_id).update_all(model_id: external_id) + llm_connection.feature_bindings.where(model_id: previous_external_id).update_all(model_id: external_id) end # The counterpart of the rename. Verdicts are keyed by the identifier string, diff --git a/app/services/llm/runtime.rb b/app/services/llm/runtime.rb new file mode 100644 index 000000000000..1c2d9027bab2 --- /dev/null +++ b/app/services/llm/runtime.rb @@ -0,0 +1,162 @@ +# 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 Llm + # Answers "which model should this feature use, and can it run right now?". + # + # The single place model resolution happens, so that every feature agrees on + # what an unset value means: + # + # per-item override -> feature binding -> connection default -> unbound + # + # A blank value at any level means "inherit from the level below". + class Runtime + # :ready - go ahead + # :feature_disabled - the feature's own toggle is off + # :no_connection - no LLM server configured, or AI switched off globally + # :unbound - nothing has chosen a model for this feature yet + # :model_missing - the chosen model is not in the server's catalogue + # :incapable - the chosen model is known not to support what is needed + Resolution = Data.define(:feature, :connection, :model_id, :status, :missing_capabilities) do + def ready? = status == :ready + + # A chat builder for the resolved model. + # + # Note that RubyLLM enforces none of the feature's declared requirements: + # Chat#with_schema performs no capability check, and a model absent from + # RubyLLM's registry is described by Model::Info.default, which claims + # structured output, vision and function calling for everything. The + # capability verdicts consulted in #call above are the only real gate. + # + # @return [RubyLLM::Chat] + def chat(**) + ensure_usable!(:chat) + session(**).chat(model_id) + end + + # @return [RubyLLM::Embedding] + def embed(input, dimensions: nil, **) + ensure_usable!(:embedding) + session(**).embed(input, model: model_id, dimensions:) + end + + # @return [Llm::Session] + def session(**) + Llm::Session.for(connection, **) + end + + private + + # Features are resolved by kind, so asking a chat feature to embed means a + # caller has confused two features -- a bug, not a configuration problem. + def ensure_usable!(kind) + raise Llm::Errors::NotReady, status unless ready? + return if feature.public_send(:"#{kind}?") + + raise Llm::Errors::NotReady, :wrong_kind + end + end + + class << self + # @param feature_key [Symbol] a key registered with OpenProject::Llm::Features + # @param override [String, nil] a per-item model choice, e.g. one stored on + # a description assistant action. Blank means inherit. + def for(feature_key, override: nil) + new(OpenProject::Llm::Features[feature_key], override:).call + end + end + + def initialize(feature, override: nil) + @feature = feature + @override = override + end + + def call + return resolution(:feature_disabled) unless feature.available? + return resolution(:no_connection) unless LlmConnection.available? + + model_id = resolved_model_id + return resolution(:unbound) if model_id.blank? + return resolution(:model_missing, model_id:) unless connection.available_model_ids.include?(model_id) + + missing = unsupported_capabilities(model_id) + return resolution(:incapable, model_id:, missing_capabilities: missing) if missing.any? + + resolution(:ready, model_id:) + end + + private + + attr_reader :feature, :override + + def connection + @connection ||= LlmConnection.instance + end + + def resolved_model_id + effective_override || binding_model_id || connection_default + end + + # A pinned feature declared overridable: false must not follow a caller's + # override: semantic_search's vectors were written with one model, and a + # different one at query time is silently wrong answers, not a preference. + def effective_override + return unless feature.overridable + + override.presence + end + + def binding_model_id + connection.feature_bindings.find_by(feature_key: feature.key.to_s)&.model_id.presence + end + + def connection_default + feature.embedding? ? connection.default_embedding_model_id : connection.default_chat_model_id + end + + # Only a definite :unsupported blocks. An :unknown verdict -- which is the + # normal state for a server that reports nothing about its models -- is + # surfaced in the UI as a warning but never prevents a call. + def unsupported_capabilities(model_id) + return [] if feature.requires.empty? + + blocking = connection.capability_verdicts + .for_model(model_id) + .where(capability: feature.requires.map(&:to_s), state: "unsupported") + + blocking.pluck(:capability).map(&:to_sym) + end + + def resolution(status, model_id: nil, missing_capabilities: []) + Resolution.new(feature:, connection: status == :feature_disabled ? nil : connection, + model_id:, status:, missing_capabilities:) + end + end +end diff --git a/app/services/llm_connections/selectable_models_query.rb b/app/services/llm_connections/selectable_models_query.rb new file mode 100644 index 000000000000..1ca7d8a4a974 --- /dev/null +++ b/app/services/llm_connections/selectable_models_query.rb @@ -0,0 +1,96 @@ +# 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 models offerable to a feature, each with why it is or is not usable. + # + # Models are never hidden. Hiding one produces the single support question + # nobody can answer -- "why can I not pick the model I know works" -- and it is + # exactly wrong when most verdicts are unknown. Instead each option carries a + # state the UI renders: selectable, selectable with a warning, or disabled with + # a reason. + class SelectableModelsQuery + Option = Data.define(:model_id, :state, :reasons) do + def selectable? = state != :unsupported + + def warning? = state == :unknown + end + + def initialize(connection, feature) + @connection = connection + @feature = feature + end + + def call + offerable_model_ids.map { |model_id| option_for(model_id) } + end + + private + + attr_reader :connection, :feature + + # Models an administrator has switched off are not offered, but the one this + # feature is already bound to stays listed -- otherwise the select silently + # shows nothing where a working binding exists. + def offerable_model_ids + (connection.selectable_model_ids + [bound_model_id]).compact_blank.uniq + end + + def bound_model_id + connection.feature_bindings.find_by(feature_key: feature.key.to_s)&.model_id + end + + def option_for(model_id) + states = feature.requires.index_with { |capability| verdict_state(model_id, capability) } + + if states.value?(:unsupported) + Option.new(model_id:, state: :unsupported, + reasons: states.select { |_, s| s == :unsupported }.keys) + elsif states.value?(:unknown) + Option.new(model_id:, state: :unknown, + reasons: states.select { |_, s| s == :unknown }.keys) + else + Option.new(model_id:, state: :supported, reasons: []) + end + end + + # No verdict at all is the same as an inconclusive one: we do not know. + def verdict_state(model_id, capability) + verdicts.dig(model_id, capability.to_s)&.to_sym || :unknown + end + + def verdicts + @verdicts ||= connection.capability_verdicts + .pluck(:model_id, :capability, :state) + .group_by(&:first) + .transform_values { |rows| rows.to_h { |(_, capability, state)| [capability, state] } } + end + end +end diff --git a/app/views/admin/llm_feature_bindings/index.html.erb b/app/views/admin/llm_feature_bindings/index.html.erb new file mode 100644 index 000000000000..7c026aac983d --- /dev/null +++ b/app/views/admin/llm_feature_bindings/index.html.erb @@ -0,0 +1,63 @@ +<%#-- 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. + +++#%> + +<% html_title t(:label_administration), t("menus.admin.llm_feature_bindings") %> + +<%= + render(Primer::OpenProject::PageHeader.new) do |header| + header.with_title { t("menus.admin.llm_feature_bindings") } + header.with_description { t(".description") } + header.with_breadcrumbs( + [{ href: admin_index_path, text: t(:label_administration) }, + { href: mcp_configurations_path, text: t("menus.admin.ai") }, + t("menus.admin.llm_feature_bindings")] + ) + end +%> + +<% if !@connection.configured? %> + <%= + render(Primer::Beta::Blankslate.new(border: true)) do |component| + component.with_visual_icon(icon: :sparkle) + component.with_heading(tag: :h2) { t(".blank_title") } + component.with_description { t(".blank_description") } + component.with_primary_action(href: llm_connection_path) { t("menus.admin.llm_connection") } + end + %> +<% else %> + <% @features.each do |feature| %> + <%= render( + LlmConnections::FeatureBindingComponent.new( + feature:, + connection: @connection, + binding: @bindings[feature.key.to_s] + ) + ) %> + <% end %> +<% end %> diff --git a/config/initializers/llm_features.rb b/config/initializers/llm_features.rb new file mode 100644 index 000000000000..568d977063fa --- /dev/null +++ b/config/initializers/llm_features.rb @@ -0,0 +1,52 @@ +# 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_relative "../../lib_static/open_project/llm/features" + +# Features that send requests to the configured LLM server. +# +# Add a feature here (or from a module engine initializer) so that +# administrators can assign it a model on the "AI models" page. + +# The description assistant rewrites work package text on explicit user action. +# Plain chat completions only: no tools, no JSON mode, no streaming. Individual +# actions may override the model, which is why it is overridable. +OpenProject::Llm::Features.register :description_assistant, + kind: :chat, + prefers: %i[structured_output], + overridable: true + +# Semantic search embeds work packages into a pgvector index. Pinned because the +# stored vectors are meaningless under a different model: changing it is a +# destructive re-index rather than a swap. +OpenProject::Llm::Features.register :semantic_search, + kind: :embedding, + requires: %i[embeddings], + pinned: true diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index bed2ac6b42cc..2872ac21a09a 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -510,6 +510,12 @@ caption: I18n.t("menus.admin.llm_connection"), parent: :ai + menu.push :llm_feature_bindings, + { controller: "/admin/llm_feature_bindings", action: :index }, + if: ->(_) { User.current.admin? && OpenProject::FeatureDecisions.llm_connection_active? }, + caption: I18n.t("menus.admin.llm_feature_bindings"), + parent: :ai + menu.push :mcp_configurations, { controller: "/admin/mcp_configurations", action: :index }, if: ->(_) { User.current.admin? }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 36f7c21ea2ee..bef2bf30d717 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -212,7 +212,15 @@ en: # 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" + default_embedding_model: "Default embedding model" llm_features_enabled: "Enable LLMs for this instance" + llm_feature_binding: + dimensions: "Dimensions" + feature_key: "Feature" + input_prefix: "Document prefix" + # ActiveRecord::Base.human_attribute_name strips the _id suffix. + model: "Model" + query_prefix: "Query prefix" llm_model: display_name: "Display name" # human_attribute_name strips the _id suffix, so the key omits it. @@ -710,6 +718,21 @@ en: 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." + llm_feature_binding: + attributes: + dimensions: + locked: "cannot be changed while data indexed with it still exists. Re-index to change it." + not_for_chat_feature: "only applies to features that create embeddings." + feature_key: + not_registered: "does not belong to a known AI feature." + input_prefix: + locked: "cannot be changed while data indexed with it still exists. Re-index to change it." + not_for_chat_feature: "only applies to features that create embeddings." + model_id: + locked: "cannot be changed while data indexed with it still exists. Re-index to switch models." + query_prefix: + locked: "cannot be changed while data indexed with it still exists. Re-index to change it." + not_for_chat_feature: "only applies to features that create embeddings." meeting: error_conflict: "Unable to save because the meeting was updated by someone else in the meantime. Please reload the page." member: @@ -1726,6 +1749,7 @@ en: configured_from_env: "This connection is configured through the environment and cannot be changed here." description: "OpenProject will stop using the LLM server. AI features will be unavailable until you connect again." heading: "Disconnect from the LLM server?" + keeps_bindings: "The model chosen for each feature is kept: %{features}." keeps_models: "The model list, including any models you added manually, is kept." keeps_settings: "The endpoint and API format are kept. Only the stored API key is removed." menu_label: "Disconnect" @@ -1754,6 +1778,27 @@ en: disabled: "Saved. LLM features are switched off for this instance." no_models: "Saved, but no models are stored yet. Either the server offers no model list, or the endpoint is missing its API version segment (for example /v1). Refresh the list on the LLMs tab." success: "Successfully connected to the LLM server." + llm_feature_bindings: + dangling: "%{model} is no longer offered by the LLM server. This feature will not run until another model is selected." + deactivated: "%{model} has been hidden by an administrator. This feature keeps using it, but it can no longer be chosen elsewhere." + form: + dimensions_caption: "How many numbers each vector has. Leave blank to use whatever the server returns." + dimensions_caption_probed: "The server returned %{dimensions}-dimension vectors for this model. Leave blank to use whatever it returns at index time." + input_prefix_caption: "Prepended to each document before it is indexed. Some models expect one, for example \"passage: \" including the trailing space." + query_prefix_caption: "Prepended to each search query. Some models expect one, for example \"query: \" including the trailing space." + index: + blank_description: "Connect OpenProject to an LLM server first. Models can be assigned once the server reports which ones it offers." + blank_title: "No LLM server configured" + description: "Choose which model each AI feature uses. Features without a choice use the instance default." + inherit_with_default: "Use the default (%{model})" + inherit_without_default: "Use the default (none set)" + locked: "%{model} is in use by indexed data and cannot be changed here. Re-index to switch models." + locked_values_heading: "Values fixed by the existing index" + option_unknown: "%{model} — not verified" + option_unsupported: "%{model} — no %{capability} support" + update: + model_incapable: "The model for %{feature} has been saved, but the server just reported that it does not support %{capability}. Pick a different model, or assert the capability on the model if you know better." + success: "The model for %{feature} has been saved." llm_models: create: success: "%{model} has been added." @@ -4354,6 +4399,13 @@ en: context_window_sources: registry: "the figure published for this model" server: "reported by the server" + features: + description_assistant: + caption: "Rewrites and restructures work package text on request." + label: "Description assistant" + semantic_search: + caption: "Indexes work packages so they can be found by meaning rather than by keyword." + label: "Semantic search" model_kinds: chat: "Chat" embedding: "Embedding" @@ -4607,6 +4659,7 @@ en: ai: "Artificial Intelligence (AI)" api_and_webhooks: "API and webhooks" llm_connection: "LLM settings" + llm_feature_bindings: "AI models" mail_notification: "Email notifications" mails_and_notifications: "Emails and notifications" mcp_configurations: "Model Context Protocol (MCP)" diff --git a/config/routes.rb b/config/routes.rb index 4d0fab08987e..3bf86f6fd83e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -762,6 +762,10 @@ end end + # Keyed by feature key rather than by record id: the binding is an attribute + # of a registered feature, and a feature may not have a row yet. + resources :llm_feature_bindings, only: %i[index update], controller: "admin/llm_feature_bindings" + resources :mcp_configurations, only: %i[index update], controller: "admin/mcp_configurations" do collection do post :multi_update diff --git a/db/migrate/20260811140100_create_llm_feature_bindings.rb b/db/migrate/20260811140100_create_llm_feature_bindings.rb new file mode 100644 index 000000000000..b9bbc61683e1 --- /dev/null +++ b/db/migrate/20260811140100_create_llm_feature_bindings.rb @@ -0,0 +1,52 @@ +# 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. +#++ + +class CreateLlmFeatureBindings < ActiveRecord::Migration[8.1] + def change + create_table :llm_feature_bindings do |t| + t.references :llm_connection, null: false, foreign_key: true + t.string :feature_key, null: false + # NULL means "use the connection default for this kind of model". + t.string :model_id + # Embedding features only. Frozen together with model_id once vectors exist. + t.integer :dimensions + t.string :input_prefix + t.string :query_prefix + # Set once the binding has data depending on it, after which the model + # cannot be swapped without a destructive re-index. + t.datetime :locked_at + t.datetime :last_seen_at + + t.timestamps null: false + end + + add_index :llm_feature_bindings, %i[llm_connection_id feature_key], unique: true + end +end diff --git a/lib_static/open_project/llm/features.rb b/lib_static/open_project/llm/features.rb new file mode 100644 index 000000000000..5165d24ca2e8 --- /dev/null +++ b/lib_static/open_project/llm/features.rb @@ -0,0 +1,120 @@ +# 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 OpenProject + module Llm + class UnknownFeature < StandardError; end + + # A feature that sends requests to the configured LLM server. + # + # Features declare the capabilities they need so the administration UI can + # tell an administrator which models are usable for which job, and so a + # feature never silently runs against a model that cannot serve it. + Feature = Data.define(:key, :kind, :requires, :prefers, :overridable, :pinned, :available, :i18n_scope) do + def available? = available.call + + def chat? = kind == :chat + + def embedding? = kind == :embedding + + def label = I18n.t("label", scope: i18n_scope) + + def caption = I18n.t("caption", scope: i18n_scope, default: nil) + end + + # The registry of LLM-consuming features. + # + # Lives in lib_static because it is populated from initializers, which run + # before eager loading; constants defined under app/ would be unloaded on a + # development reload and lose their registrations. This is the same reason + # OpenProject::FeatureDecisions lives here. + # + # Register from config/initializers/llm_features.rb for core features, or + # from a module's engine: + # + # initializer "openproject_foo.llm_features" do + # OpenProject::Llm::Features.register :foo, kind: :chat + # end + module Features + module_function + + KINDS = %i[chat embedding].freeze + + # Mirrors Llm::Capabilities, which owns the vocabulary and knows how to + # read published values from the model registry. Duplicated as literals + # here because lib_static is autoloaded once, before app/ is available. + CAPABILITIES = { + chat: %i[function_calling structured_output vision reasoning].freeze, + embedding: %i[embeddings].freeze + }.freeze + + def register(key, + kind:, + requires: [], + prefers: [], + overridable: false, + pinned: false, + available: -> { true }, + i18n_scope: nil) + key = key.to_sym + validate!(key, kind, requires + prefers) + + all[key] = Feature.new(key:, kind:, requires: requires.map(&:to_sym).freeze, + prefers: prefers.map(&:to_sym).freeze, + overridable:, pinned:, available:, + i18n_scope: i18n_scope || "llm.features.#{key}") + end + + def all = @all ||= {} + + def [](key) + all.fetch(key.to_sym) { raise UnknownFeature, key.to_s } + end + + def registered?(key) = all.key?(key.to_sym) + + # Features whose own toggle is on. A feature that is switched off keeps its + # stored binding: flipping a flag must not lose an administrator's choice. + def available = all.values.select(&:available?) + + def for_kind(kind) = available.select { |feature| feature.kind == kind } + + def validate!(key, kind, capabilities) + raise ArgumentError, "unknown kind #{kind.inspect}" unless KINDS.include?(kind) + raise ArgumentError, "LLM feature #{key} is already registered" if all.key?(key) + + unknown = capabilities.map(&:to_sym) - CAPABILITIES.fetch(kind) + return if unknown.empty? + + raise ArgumentError, "#{unknown.join(', ')} not valid for a #{kind} feature" + end + end + end +end diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index 20ebc087c244..992e57d4b635 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -232,4 +232,18 @@ def choose_action(item) expect(connection.models.count).to eq(2) end end + + describe "the AI models page" do + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + + before { mock_llm_embeddings_response(base_url) } + + it "offers the vector settings only for features that embed" do + visit llm_feature_bindings_path + + expect(page).to have_test_selector("llm-feature-binding--dimensions-semantic_search") + expect(page).to have_no_test_selector("llm-feature-binding--dimensions-description_assistant") + expect(page).to be_axe_clean.within("#content") + end + end end diff --git a/spec/models/llm_model_spec.rb b/spec/models/llm_model_spec.rb index 98ba190daeb7..7b0aeb5bd037 100644 --- a/spec/models/llm_model_spec.rb +++ b/spec/models/llm_model_spec.rb @@ -68,8 +68,23 @@ expect(connection.selectable_model_ids).to include("bge-m3") end + # The decision that makes the toggle safe: curation, not enforcement. A row an + # administrator switches off must never silently break a running feature. it "stays addressable for a feature that is already bound to it" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + expect(connection.available_model_ids).to include("qwen3.6-27b") + expect(Llm::Runtime.for(:description_assistant)).to be_ready + end + + it "still offers it to the feature that is bound to it" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + + options = LlmConnections::SelectableModelsQuery + .new(connection, OpenProject::Llm::Features[:description_assistant]) + .call + + expect(options.map(&:model_id)).to include("qwen3.6-27b") end # The reason deactivated_at exists rather than reusing active: the sync writes diff --git a/spec/requests/admin/llm_connections_spec.rb b/spec/requests/admin/llm_connections_spec.rb index 3bc2dd3ae7b1..2c4f46e6ee42 100644 --- a/spec/requests/admin/llm_connections_spec.rb +++ b/spec/requests/admin/llm_connections_spec.rb @@ -375,14 +375,19 @@ end it "offers the confirmation, naming what is kept" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + get disconnect_dialog_llm_connection_path, headers: { "Accept" => "text/vnd.turbo-stream.html" } expect(response).to have_http_status(:ok) expect(response.body).to include("Disconnect from the LLM server?") + expect(response.body).to include("Description assistant") end it "clears the credential and switches the connection off, keeping everything else" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + post disconnect_llm_connection_path connection.reload @@ -390,6 +395,7 @@ expect(Setting.llm_features_enabled?).to be(false) expect(connection.base_url).to eq("https://example.com/v1") expect(connection.models.count).to eq(2) + expect(connection.feature_bindings.first.model_id).to eq("qwen3.6-27b") end it "refuses when the connection comes from the environment" do diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb new file mode 100644 index 000000000000..73139246a5c8 --- /dev/null +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -0,0 +1,186 @@ +# 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 "Admin AI model assignment", :llm_server_helpers, :skip_csrf, :webmock, + type: :rails_request, with_flag: { llm_connection: true } do + let(:admin) { create(:admin) } + let(:base_url) { "https://example.com/v1" } + + describe "GET /admin/llm_feature_bindings" do + before { login_as admin } + + it "prompts to configure a connection when there is none" do + get llm_feature_bindings_path + + expect(response).to have_http_status(:ok) + expect(response.body).to include("No LLM server configured") + end + + context "with a configured connection" do + let!(:connection) { create(:llm_connection, :with_models, base_url:) } + + it "lists every registered feature" do + get llm_feature_bindings_path + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Description assistant") + expect(response.body).to include("Semantic search") + end + + # Hiding an unusable model is the one thing that produces an unanswerable + # support question, so it stays listed and says why it cannot be chosen. + it "offers a model with no verdict, marked as unverified" do + get llm_feature_bindings_path + + expect(response.body).to include("qwen3.6-27b — not verified") + end + + it "disables a model known not to support a required capability" do + connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", + state: "unsupported", source: "probe", checked_at: Time.current) + + get llm_feature_bindings_path + + expect(response.body).to include("qwen3.6-27b — no Embeddings support") + end + end + end + + describe "PATCH /admin/llm_feature_bindings/:id" do + let!(:connection) { create(:llm_connection, :with_models, base_url:) } + + before { login_as admin } + + it "stores the chosen model" do + patch llm_feature_binding_path("description_assistant"), + params: { llm_feature_binding: { model_id: "qwen3.6-27b" } } + + expect(response).to have_http_status(:see_other) + expect(connection.feature_bindings.find_by(feature_key: "description_assistant").model_id) + .to eq("qwen3.6-27b") + end + + it "treats a blank choice as inheriting the default" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + + patch llm_feature_binding_path("description_assistant"), params: { llm_feature_binding: { model_id: "" } } + + expect(connection.feature_bindings.find_by(feature_key: "description_assistant").model_id).to be_nil + end + + # The verdict that matters is the one for the model just chosen, so it is + # fetched now rather than left unknown until the feature first runs. + it "probes the model when the feature requires a capability" do + request = stub_request(:post, "#{base_url}/embeddings") + .to_return(status: 200, + headers: { "Content-Type" => "application/json" }, + body: { data: [{ embedding: [0.1, 0.2] }] }.to_json) + + patch llm_feature_binding_path("semantic_search"), params: { llm_feature_binding: { model_id: "bge-m3" } } + + expect(request).to have_been_made.once + verdict = connection.capability_verdicts.find_by(model_id: "bge-m3", capability: "embeddings") + expect(verdict.state).to eq("supported") + expect(verdict.dimensions).to eq(2) + end + + it "does not probe for a feature that requires nothing" do + patch llm_feature_binding_path("description_assistant"), + params: { llm_feature_binding: { model_id: "qwen3.6-27b" } } + + expect(a_request(:post, "#{base_url}/embeddings")).not_to have_been_made + end + + it "404s for a feature that is not registered" do + patch llm_feature_binding_path("no_such_feature"), params: { llm_feature_binding: { model_id: "x" } } + + expect(response).to have_http_status(:not_found) + end + end + + describe "embedding settings" do + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + + before do + login_as admin + # Binding an embedding feature probes the model for a vector. + mock_llm_embeddings_response(base_url) + end + + it "stores the vector settings, keeping the prefixes exactly as typed" do + patch llm_feature_binding_path(:semantic_search), + params: { llm_feature_binding: { model_id: "bge-m3", + dimensions: "1024", + input_prefix: "passage: ", + query_prefix: "query: " } } + + binding = connection.feature_bindings.find_by(feature_key: "semantic_search") + + expect(binding.dimensions).to eq(1024) + # The trailing space is load-bearing for the E5 and BGE families. + expect(binding.input_prefix).to eq("passage: ") + expect(binding.query_prefix).to eq("query: ") + end + + it "rejects a dimension count that is not a positive integer" do + patch llm_feature_binding_path(:semantic_search), + params: { llm_feature_binding: { model_id: "bge-m3", dimensions: "0" } } + + expect(connection.feature_bindings.find_by(feature_key: "semantic_search")&.dimensions).to be_nil + end + + it "ignores vector settings sent to a chat feature" do + patch llm_feature_binding_path(:description_assistant), + params: { llm_feature_binding: { model_id: "qwen3.6-27b", dimensions: "1024" } } + + binding = connection.feature_bindings.find_by(feature_key: "description_assistant") + + expect(binding.model_id).to eq("qwen3.6-27b") + expect(binding.dimensions).to be_nil + end + + # A locked binding is the record that a vector index exists. Everything the + # index depends on is frozen, not just the model. + it "refuses to change anything a locked index depends on" do + binding = connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "bge-m3", + dimensions: 1024, input_prefix: "passage: ", + locked_at: Time.current) + + patch llm_feature_binding_path(:semantic_search), + params: { llm_feature_binding: { model_id: "bge-m3", dimensions: "512", input_prefix: "other: " } } + + binding.reload + expect(binding.dimensions).to eq(1024) + expect(binding.input_prefix).to eq("passage: ") + end + end +end diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index b5ff48361fdc..0213935be4e0 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -330,6 +330,16 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(connection.models.where(external_id: "already-there").count).to eq(1) end + + it "makes the model bindable straight away" do + post llm_models_path, params: { llm_model: { external_id: "qwen3.6-35b-a3b" } } + + patch llm_feature_binding_path("description_assistant"), + params: { llm_feature_binding: { model_id: "qwen3.6-35b-a3b" } } + + expect(connection.feature_bindings.find_by(feature_key: "description_assistant").model_id) + .to eq("qwen3.6-35b-a3b") + end end describe "a refresh that cannot see the manual model" do @@ -399,6 +409,15 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(llm_model.context_window_source).to eq(:server) end + it "makes an asserted type satisfy a feature that requires it" do + patch llm_model_path(llm_model), params: { llm_model: { model_type: "embedding" } } + + patch llm_feature_binding_path("semantic_search"), + params: { llm_feature_binding: { model_id: "hand-typed" } } + + expect(connection.feature_bindings.find_by(feature_key: "semantic_search").model_id).to eq("hand-typed") + end + # Clearing an assertion records nothing rather than recording ignorance as # fact, so detection can still fill it in later. it "clears an assertion when set back to unspecified" do @@ -559,6 +578,21 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size end end + describe "GET /admin/llm_models/:id/delete_dialog" do + it "offers a confirmation naming the features that would break" do + llm_model = create(:llm_model, :manual, llm_connection: connection, external_id: "hand-typed") + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "hand-typed") + + # Requested by the async-dialog Stimulus controller, which asks for a + # turbo stream rather than HTML. + get delete_dialog_llm_model_path(llm_model), + headers: { "Accept" => "text/vnd.turbo-stream.html" } + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Description assistant") + end + end + describe "renaming a manually added model" do let!(:llm_model) do create(:llm_model, :manual, llm_connection: connection, external_id: "qwen/qwen3.6-35b-a3b") @@ -566,6 +600,8 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size before do connection.update!(default_chat_model: llm_model) + connection.feature_bindings.create!(feature_key: "description_assistant", + model_id: "qwen/qwen3.6-35b-a3b") connection.capability_verdicts.create!(model_id: "qwen/qwen3.6-35b-a3b", capability: "embeddings", state: "unsupported", source: "probe", checked_at: Time.current) end @@ -593,9 +629,27 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(llm_model.reload.external_id).to eq("qwen/qwen3.6-35b-a3b:bf16") expect(connection.reload.default_chat_model).to eq(llm_model) + expect(connection.feature_bindings.first.model_id).to eq("qwen/qwen3.6-35b-a3b:bf16") expect(connection.capability_verdicts.first.model_id).to eq("qwen/qwen3.6-35b-a3b:bf16") end + it "keeps the feature resolving afterwards", with_flag: { llm_connection: true } do + connection.update!(enabled: true) + + patch llm_model_path(llm_model), params: { llm_model: { external_id: "qwen/qwen3.6-35b-a3b:bf16" } } + + expect(Llm::Runtime.for(:description_assistant).model_id).to eq("qwen/qwen3.6-35b-a3b:bf16") + end + + it "follows a model a locked binding depends on" do + binding = connection.feature_bindings.first + binding.update!(locked_at: Time.current) + + patch llm_model_path(llm_model), params: { llm_model: { external_id: "qwen/qwen3.6-35b-a3b:bf16" } } + + expect(binding.reload.model_id).to eq("qwen/qwen3.6-35b-a3b:bf16") + end + # The server names its own models; renaming one here would only be undone by # the next refresh. it "refuses to rename a discovered model" do @@ -717,6 +771,16 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(offered_default_models(streamed_markup)).not_to include("qwen3.6-27b") end + # Curation, not enforcement: a feature already pointing at the model keeps + # resolving, so switching a row off cannot silently break anything. + it "leaves an existing binding working" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") + + post toggle_llm_model_path(llm_model) + + expect(connection.available_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") diff --git a/spec/services/llm/runtime_spec.rb b/spec/services/llm/runtime_spec.rb new file mode 100644 index 000000000000..d462972431e7 --- /dev/null +++ b/spec/services/llm/runtime_spec.rb @@ -0,0 +1,176 @@ +# 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 Llm::Runtime, with_flag: { llm_connection: true } do + subject(:resolution) { described_class.for(feature_key, override:) } + + let(:feature_key) { :description_assistant } + let(:override) { nil } + + context "without a connection" do + it { expect(resolution.status).to eq(:no_connection) } + end + + context "with a connection that is not enabled" do + before { create(:llm_connection, :with_models, enabled: false) } + + it { expect(resolution.status).to eq(:no_connection) } + end + + context "with an enabled connection" do + let!(:connection) { create(:llm_connection, :with_models, :enabled) } + + it "is unbound until a model is chosen" do + expect(resolution.status).to eq(:unbound) + expect(resolution.model_id).to be_nil + end + + it "falls back to the connection default" do + connection.update!(default_chat_model_id: "qwen3.6-27b") + + expect(resolution).to be_ready + expect(resolution.model_id).to eq("qwen3.6-27b") + end + + it "prefers the feature binding over the connection default" do + connection.update!(default_chat_model_id: "qwen3.6-27b") + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "bge-m3") + + expect(resolution.model_id).to eq("bge-m3") + end + + context "with a per-item override" do + let(:override) { "qwen3.6-27b" } + + it "wins over the binding" do + connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "bge-m3") + + expect(resolution.model_id).to eq("qwen3.6-27b") + end + + # semantic_search's vectors were written with the bound model; a different + # one at query time is silently wrong answers, not a preference. + it "is ignored by a feature that is not overridable" do + connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "bge-m3") + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "admin", checked_at: Time.current) + + resolution = described_class.for(:semantic_search, override: "qwen3.6-27b") + + expect(resolution.model_id).to eq("bge-m3") + end + end + + # Substituting the default here would silently change the output of a + # transform an administrator configured deliberately. + context "when the chosen model is gone from the catalogue" do + let(:override) { "vanished-model" } + + before { connection.update!(default_chat_model_id: "qwen3.6-27b") } + + it "fails closed rather than falling back" do + expect(resolution.status).to eq(:model_missing) + expect(resolution.model_id).to eq("vanished-model") + end + end + end + + describe "capability gating" do + let(:feature_key) { :semantic_search } + let!(:connection) { create(:llm_connection, :with_models, :enabled) } + + before { connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "qwen3.6-27b") } + + it "blocks on a definite unsupported verdict" do + connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", + state: "unsupported", source: "probe", checked_at: Time.current) + + expect(resolution.status).to eq(:incapable) + expect(resolution.missing_capabilities).to eq([:embeddings]) + end + + # Refusing on "we could not tell" would make most self-hosted servers + # unusable, since the model list carries no capability information at all. + it "does not block when the verdict is unknown" do + connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", + state: "unknown", source: "probe", checked_at: Time.current) + + expect(resolution).to be_ready + end + + it "does not block when there is no verdict at all" do + expect(resolution).to be_ready + end + end + + describe "running a request", :llm_server_helpers, :webmock do + let!(:connection) { create(:llm_connection, :with_models, :enabled, default_chat_model_id: "qwen3.6-27b") } + + it "sends a completion for the resolved model" do + mock_llm_chat_response("https://example.com/v1", content: "pong") + + expect(resolution.chat(max_retries: 0).ask("ping").content).to eq("pong") + expect(WebMock).to have_requested(:post, "https://example.com/v1/chat/completions") + .with(body: hash_including("model" => "qwen3.6-27b")) + end + + it "refuses when the feature is not ready" do + connection.update!(enabled: false) + + expect { resolution.chat }.to raise_error(Llm::Errors::NotReady) { |e| expect(e.status).to eq(:no_connection) } + end + + # Features are resolved by kind, so asking a chat feature to embed means a + # caller has confused two features. + it "refuses to embed through a chat feature" do + expect { resolution.embed("hello") } + .to raise_error(Llm::Errors::NotReady) { |e| expect(e.status).to eq(:wrong_kind) } + end + + context "with an embedding feature" do + let(:feature_key) { :semantic_search } + + before { connection.update!(default_embedding_model_id: "bge-m3") } + + it "requests a vector for the resolved model" do + mock_llm_embeddings_response("https://example.com/v1", dimensions: 8) + + expect(resolution.embed("hello", max_retries: 0).vectors.length).to eq(8) + end + + it "refuses to chat through an embedding feature" do + expect { resolution.chat } + .to raise_error(Llm::Errors::NotReady) { |e| expect(e.status).to eq(:wrong_kind) } + end + end + end +end From aca64b49171982b1fa2c14a3431ef5d8e87cf861 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 10:05:00 +0200 Subject: [PATCH 02/10] [AI-3] Add the default embedding model to the AI models page The second default picker joins the chat one in the Default models section, so both defaults are chosen where model availability is controlled, as agreed in the UX review with Tom. It offers only models known to create embeddings, keeps a stored choice listed and flagged so a save cannot blank it, and points at the documentation about model types for administrators who do not know what an embedding model is. The contract now judges the embedding default by the model type, the same rule the picker offers by, so form, table and contract agree. The bindings page becomes Feature configuration and its description points at the AI models page where the defaults now live. --- .../llm_connections/base_contract.rb | 13 ++--- .../admin/llm_models_controller.rb | 2 +- .../llm_connections/default_models_form.rb | 56 +++++++++++++++++++ app/models/llm_connection.rb | 4 ++ .../admin/llm_feature_bindings/index.html.erb | 6 +- config/initializers/llm_features.rb | 2 +- config/locales/en.yml | 9 ++- spec/features/admin/llm_connection_spec.rb | 9 +-- spec/requests/admin/llm_connections_spec.rb | 7 ++- .../admin/llm_feature_bindings_spec.rb | 10 +++- spec/requests/admin/llm_models_spec.rb | 56 +++++++++++++++++-- 11 files changed, 149 insertions(+), 25 deletions(-) diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index 62aac5aec963..c32f3cfd46e0 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -65,20 +65,17 @@ def not_configured_from_env private - # A model the server has positively told us cannot embed is not a candidate - # for the embedding default, however it got submitted. An unknown verdict - # does not block: that is the normal state for a server that publishes - # nothing about its models. + # Only a model known to embed can serve the embedding default, the same rule + # the picker offers by. A model the connection does not know at all is left + # to default_models_offered_by_server. def default_embedding_model_can_embed model_id = model.default_embedding_model_id return if model_id.blank? return unless model.changed_attributes.include?("default_embedding_model_id") - unsupported = model.capability_verdicts - .for_capability(:embeddings) - .exists?(model_id:, state: "unsupported") + llm_model = model.models.find_by(external_id: model_id) - errors.add(:default_embedding_model_id, :cannot_embed) if unsupported + errors.add(:default_embedding_model_id, :cannot_embed) if llm_model && !llm_model.embedding? end # A model the server identifies as an embedding model is not a chat candidate. diff --git a/app/controllers/admin/llm_models_controller.rb b/app/controllers/admin/llm_models_controller.rb index 39e813fd97ad..c002df163b69 100644 --- a/app/controllers/admin/llm_models_controller.rb +++ b/app/controllers/admin/llm_models_controller.rb @@ -155,7 +155,7 @@ def toggle private def default_model_params - params.expect(llm_connection: %i[default_chat_model_id]).to_h.symbolize_keys + params.expect(llm_connection: %i[default_chat_model_id default_embedding_model_id]).to_h.symbolize_keys end def set_connection diff --git a/app/forms/llm_connections/default_models_form.rb b/app/forms/llm_connections/default_models_form.rb index a8a049c54ed4..a132427c25c5 100644 --- a/app/forms/llm_connections/default_models_form.rb +++ b/app/forms/llm_connections/default_models_form.rb @@ -30,6 +30,8 @@ module LlmConnections class DefaultModelsForm < ApplicationForm + include Redmine::I18n + 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 @@ -55,6 +57,29 @@ class DefaultModelsForm < ApplicationForm end end + # Only worth asking for once something embeds. + if embedding_features? + f.autocompleter( + name: :default_embedding_model_id, + label: LlmConnection.human_attribute_name(:default_embedding_model_id), + caption: embedding_caption, + autocomplete_options: { + decorated: true, + disabled: read_only?, + inputValue: model.default_embedding_model_id, + placeholder: I18n.t("label_none_parentheses") + } + ) do |list| + list.option(label: I18n.t("label_none_parentheses"), value: "", + selected: model.default_embedding_model_id.blank?) + + default_embedding_model_options.each do |llm_model| + list.option(label: embedding_option_label(llm_model), value: llm_model.id, + selected: model.default_embedding_model_id == llm_model.id) + end + end + end + f.submit(name: :submit, label: I18n.t(:button_save), scheme: :primary) unless read_only? end @@ -69,5 +94,36 @@ def read_only? def default_chat_model_options (model.chat_models + [model.default_chat_model]).compact.uniq end + + def embedding_features? + OpenProject::Llm::Features.for_kind(:embedding).any? + end + + # Only models actually known to embed. Offering one on the grounds that + # nothing has ruled it out invites a choice whose failure surfaces much + # later, at index time. + def default_embedding_model_options + (embedding_models + [model.default_embedding_model]).compact.uniq + end + + def embedding_models + @embedding_models ||= model.embedding_models + end + + def embedding_option_label(llm_model) + return llm_model.name if embedding_models.include?(llm_model) + + I18n.t("admin.llm_models.defaults.embedding_option_unqualified", model: llm_model.name) + end + + # Says how to make a model eligible when none is, rather than leaving an + # empty picker with no explanation. + def embedding_caption + return I18n.t("admin.llm_models.defaults.embedding_none") if embedding_models.empty? + + link_translate("admin.llm_models.defaults.embedding_caption", + links: { docs_url: %i[embeddings_explanation] }, + external: true) + end end end diff --git a/app/models/llm_connection.rb b/app/models/llm_connection.rb index e6582c6934fd..41425944e38c 100644 --- a/app/models/llm_connection.rb +++ b/app/models/llm_connection.rb @@ -122,6 +122,10 @@ def chat_models selectable_models.reject(&:embedding?) end + def embedding_models + selectable_models.select(&:embedding?) + end + def embedding_model_ids embedding = capability_verdicts.for_capability(:embeddings).where(state: "supported").pluck(:model_id) diff --git a/app/views/admin/llm_feature_bindings/index.html.erb b/app/views/admin/llm_feature_bindings/index.html.erb index 7c026aac983d..499a5cb4b9c2 100644 --- a/app/views/admin/llm_feature_bindings/index.html.erb +++ b/app/views/admin/llm_feature_bindings/index.html.erb @@ -32,7 +32,11 @@ See COPYRIGHT and LICENSE files for more details. <%= render(Primer::OpenProject::PageHeader.new) do |header| header.with_title { t("menus.admin.llm_feature_bindings") } - header.with_description { t(".description") } + header.with_description do + link_translate("admin.llm_feature_bindings.index.description", + links: { models_url: llm_models_path }, + external: false) + end header.with_breadcrumbs( [{ href: admin_index_path, text: t(:label_administration) }, { href: mcp_configurations_path, text: t("menus.admin.ai") }, diff --git a/config/initializers/llm_features.rb b/config/initializers/llm_features.rb index 568d977063fa..7714446f1ff6 100644 --- a/config/initializers/llm_features.rb +++ b/config/initializers/llm_features.rb @@ -33,7 +33,7 @@ # Features that send requests to the configured LLM server. # # Add a feature here (or from a module engine initializer) so that -# administrators can assign it a model on the "AI models" page. +# administrators can assign it a model on the "Feature configuration" page. # The description assistant rewrites work package text on explicit user action. # Plain chat completions only: no tools, no JSON mode, no streaming. Individual diff --git a/config/locales/en.yml b/config/locales/en.yml index bef2bf30d717..fcc3fdb37b3d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -714,7 +714,7 @@ en: 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." + cannot_embed: "is not known to create embeddings." 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." @@ -1789,7 +1789,7 @@ en: index: blank_description: "Connect OpenProject to an LLM server first. Models can be assigned once the server reports which ones it offers." blank_title: "No LLM server configured" - description: "Choose which model each AI feature uses. Features without a choice use the instance default." + description: "Choose which model each AI feature uses. Features without a choice use the default models set on the [LLMs](models_url) tab." inherit_with_default: "Use the default (%{model})" inherit_without_default: "Use the default (none set)" locked: "%{model} is in use by indexed data and cannot be changed here. Re-index to switch models." @@ -1805,6 +1805,9 @@ en: 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." + embedding_caption: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." + embedding_none: "No model is known to create embeddings. If you know one does, open it in the list below and set its type to Embedding model." + embedding_option_unqualified: "%{model} (not known to create embeddings)" heading: "Default models" success: "The default models have been saved." destroy: @@ -4659,7 +4662,7 @@ en: ai: "Artificial Intelligence (AI)" api_and_webhooks: "API and webhooks" llm_connection: "LLM settings" - llm_feature_bindings: "AI models" + llm_feature_bindings: "Feature configuration" mail_notification: "Email notifications" mails_and_notifications: "Emails and notifications" mcp_configurations: "Model Context Protocol (MCP)" diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index 992e57d4b635..a12b223fa46c 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -49,10 +49,11 @@ # 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"] + def offered_default_models(field = :default_chat_model_id) + element = all("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter") + .find { |node| node["data-input-name"].include?(field.to_s) } - JSON.parse(items).pluck("id").compact_blank + JSON.parse(element["data-items"]).pluck("id").compact_blank end def choose_action(item) @@ -233,7 +234,7 @@ def choose_action(item) end end - describe "the AI models page" do + describe "the Feature configuration page" do let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } before { mock_llm_embeddings_response(base_url) } diff --git a/spec/requests/admin/llm_connections_spec.rb b/spec/requests/admin/llm_connections_spec.rb index 2c4f46e6ee42..8a80185d15d6 100644 --- a/spec/requests/admin/llm_connections_spec.rb +++ b/spec/requests/admin/llm_connections_spec.rb @@ -422,9 +422,12 @@ 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" } } + patch llm_connection_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b", + default_embedding_model_id: "bge-m3" } } - expect(connection.reload.default_chat_model_id).to be_nil + connection.reload + expect(connection.default_chat_model_id).to be_nil + expect(connection.default_embedding_model_id).to be_nil end end end diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index 73139246a5c8..e32dca579b54 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -30,7 +30,7 @@ require "spec_helper" -RSpec.describe "Admin AI model assignment", :llm_server_helpers, :skip_csrf, :webmock, +RSpec.describe "Admin AI feature configuration", :llm_server_helpers, :skip_csrf, :webmock, type: :rails_request, with_flag: { llm_connection: true } do let(:admin) { create(:admin) } let(:base_url) { "https://example.com/v1" } @@ -52,10 +52,18 @@ get llm_feature_bindings_path expect(response).to have_http_status(:ok) + expect(response.body).to include("Feature configuration") expect(response.body).to include("Description assistant") expect(response.body).to include("Semantic search") end + it "points at the page where the default models are chosen" do + get llm_feature_bindings_path + + expect(response.body).to include("use the default models set on the") + expect(response.body).to include(llm_models_path) + end + # Hiding an unusable model is the one thing that produces an unanswerable # support question, so it stays listed and says why it cannot be chosen. it "offers a model with no verdict, marked as unverified" do diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 0213935be4e0..da97cc684a8f 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -38,9 +38,10 @@ # The picker is an autocompleter, so its options are serialised into the # element rather than rendered as markup. - def offered_default_models(markup = page) - items = markup.find("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter")["data-items"] - ids = JSON.parse(items).pluck("id").compact_blank + def offered_default_models(field = :default_chat_model_id, markup: page) + element = markup.all("[data-test-selector='llm-connection--defaults-form'] opce-autocompleter") + .find { |node| node["data-input-name"].include?(field.to_s) } + ids = JSON.parse(element["data-items"]).pluck("id").compact_blank LlmModel.where(id: ids).pluck(:external_id) end @@ -171,6 +172,37 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) expect(page).to have_no_button("Save") end + it "offers only models known to embed as the default embedding model" do + connection = create(:llm_connection, :with_models, :enabled, 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(offered_default_models(:default_embedding_model_id)).to contain_exactly("bge-m3") + end + + # An unconfirmed capability is not a capability: offering such a model + # invites a choice that fails much later, at index time. + it "says how to make a model eligible while none is known to embed" do + create(:llm_connection, :with_models, :enabled, base_url:) + + get llm_models_path + + expect(response.body).to include("set its type to Embedding model") + end + + # Otherwise a save would silently blank a working configuration. + it "keeps the stored embedding default listed, flagged, once it is ruled out" do + connection = create(:llm_connection, :with_models, :enabled, base_url:) + connection.update_column(:default_embedding_model_id, "qwen3.6-27b") + + get llm_models_path + + expect(offered_default_models(:default_embedding_model_id)).to include("qwen3.6-27b") + expect(response.body).to include("not known to create embeddings") + 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") @@ -719,6 +751,22 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(flash[:error]).to be_present end + it "stores the default embedding model" do + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + + patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "bge-m3" } } + + expect(connection.reload.default_embedding_model_id).to eq("bge-m3") + end + + it "refuses a model that is not known to embed" do + patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "qwen3.6-27b" } } + + expect(connection.reload.default_embedding_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: chat_model.id, base_url: "https://elsewhere.test/v1" } } @@ -768,7 +816,7 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(response.media_type).to eq("text/vnd.turbo-stream.html") expect(response.body).to include('target="llm-connections-default-models-component"') - expect(offered_default_models(streamed_markup)).not_to include("qwen3.6-27b") + expect(offered_default_models(markup: streamed_markup)).not_to include("qwen3.6-27b") end # Curation, not enforcement: a feature already pointing at the model keeps From 1583f304448b43d5dac1a9becd92b729bb9d42e4 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 11:07:38 +0200 Subject: [PATCH 03/10] [AI-3] Offer embedding features only models known to embed Following the UX review with Tom, the picker of a feature offers models of the feature's own kind instead of listing every stored model with a warning or a disabled entry. Semantic search sees only models known to create embeddings, and its caption says so and links to the documentation for administrators who do not know what an embedding model is. The model a feature is already bound to stays listed and choosable even once it no longer qualifies, flagged, so that opening the page and saving it cannot blank a working binding. --- .../llm_connections/feature_binding_form.rb | 33 +++++----- .../selectable_models_query.rb | 60 ++++++------------- config/locales/en.yml | 4 +- spec/models/llm_model_spec.rb | 12 ++++ .../admin/llm_feature_bindings_spec.rb | 36 ++++++++--- 5 files changed, 79 insertions(+), 66 deletions(-) diff --git a/app/forms/llm_connections/feature_binding_form.rb b/app/forms/llm_connections/feature_binding_form.rb index cf7a50fbd5f7..4895870c7e8c 100644 --- a/app/forms/llm_connections/feature_binding_form.rb +++ b/app/forms/llm_connections/feature_binding_form.rb @@ -31,6 +31,8 @@ module LlmConnections # The model select for one registered feature. class FeatureBindingForm < ApplicationForm + include Redmine::I18n + # Primer::Forms::Base.new assigns the builder itself and calls this with the # remaining keywords, so the builder must not appear in the signature. def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: false, dimensions_hint: nil, @@ -52,6 +54,7 @@ def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: f.autocompleter( name: :model_id, label: LlmFeatureBinding.human_attribute_name(:model_id), + caption: model_caption, disabled: locked, autocomplete_options: { decorated: true, @@ -63,12 +66,9 @@ def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: list.option(label: inherit_label, value: "", selected: selected_model_id.blank?) model_options.each do |option| - # Listed but not choosable when a required capability is known to be - # missing: hiding it would leave the reason invisible too. list.option(label: option_label(option), value: option.model_id, - selected: selected_model_id == option.model_id, - disabled: !option.selectable?) + selected: selected_model_id == option.model_id) end end @@ -127,17 +127,22 @@ def dimensions_caption I18n.t("admin.llm_feature_bindings.form.dimensions_caption_probed", dimensions: dimensions_hint) end + # Says why only a few of the stored models are on offer, for an administrator + # who does not know what an embedding model is. + def model_caption + return unless embedding + + link_translate("admin.llm_feature_bindings.form.model_caption_embedding", + links: { docs_url: %i[embeddings_explanation] }, + external: true) + end + + # The bound model stays choosable once it no longer qualifies, so that saving + # the form again does not blank the binding. The label says what changed. def option_label(option) - case option.state - when :unsupported - I18n.t("admin.llm_feature_bindings.option_unsupported", - model: option.model_id, - capability: option.reasons.map { |reason| Llm::Capabilities.label(reason) }.join(", ")) - when :unknown - I18n.t("admin.llm_feature_bindings.option_unknown", model: option.model_id) - else - option.model_id - end + return option.model_id if option.qualifies + + I18n.t("admin.llm_feature_bindings.option_unqualified", model: option.model_id) end end end diff --git a/app/services/llm_connections/selectable_models_query.rb b/app/services/llm_connections/selectable_models_query.rb index 1ca7d8a4a974..f8665b270d7c 100644 --- a/app/services/llm_connections/selectable_models_query.rb +++ b/app/services/llm_connections/selectable_models_query.rb @@ -29,19 +29,18 @@ #++ module LlmConnections - # The models offerable to a feature, each with why it is or is not usable. + # The models offerable to a feature. # - # Models are never hidden. Hiding one produces the single support question - # nobody can answer -- "why can I not pick the model I know works" -- and it is - # exactly wrong when most verdicts are unknown. Instead each option carries a - # state the UI renders: selectable, selectable with a warning, or disabled with - # a reason. + # A feature is offered the models of its own kind: chat features never see an + # embedding model, and an embedding feature sees only models known to embed. + # Offering a model on the grounds that nothing has ruled it out invites a + # choice whose failure surfaces much later, at index time. + # + # The one exception is the model a feature is already bound to. It stays + # listed even once it no longer qualifies, flagged, so that opening the page + # cannot silently blank a working binding. class SelectableModelsQuery - Option = Data.define(:model_id, :state, :reasons) do - def selectable? = state != :unsupported - - def warning? = state == :unknown - end + Option = Data.define(:model_id, :qualifies) def initialize(connection, feature) @connection = connection @@ -49,48 +48,25 @@ def initialize(connection, feature) end def call - offerable_model_ids.map { |model_id| option_for(model_id) } + offerable_model_ids.map { |model_id| Option.new(model_id:, qualifies: qualifying_ids.include?(model_id)) } end private attr_reader :connection, :feature - # Models an administrator has switched off are not offered, but the one this - # feature is already bound to stays listed -- otherwise the select silently - # shows nothing where a working binding exists. def offerable_model_ids - (connection.selectable_model_ids + [bound_model_id]).compact_blank.uniq - end - - def bound_model_id - connection.feature_bindings.find_by(feature_key: feature.key.to_s)&.model_id - end - - def option_for(model_id) - states = feature.requires.index_with { |capability| verdict_state(model_id, capability) } - - if states.value?(:unsupported) - Option.new(model_id:, state: :unsupported, - reasons: states.select { |_, s| s == :unsupported }.keys) - elsif states.value?(:unknown) - Option.new(model_id:, state: :unknown, - reasons: states.select { |_, s| s == :unknown }.keys) - else - Option.new(model_id:, state: :supported, reasons: []) - end + (qualifying_ids + [bound_model_id]).compact_blank.uniq end - # No verdict at all is the same as an inconclusive one: we do not know. - def verdict_state(model_id, capability) - verdicts.dig(model_id, capability.to_s)&.to_sym || :unknown + # Models an administrator has switched off are not offered: both lists are + # built from the selectable ones. + def qualifying_ids + @qualifying_ids ||= feature.embedding? ? connection.embedding_model_ids : connection.chat_model_ids end - def verdicts - @verdicts ||= connection.capability_verdicts - .pluck(:model_id, :capability, :state) - .group_by(&:first) - .transform_values { |rows| rows.to_h { |(_, capability, state)| [capability, state] } } + def bound_model_id + connection.feature_bindings.find_by(feature_key: feature.key.to_s)&.model_id end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index fcc3fdb37b3d..973b0c6c47f3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1785,6 +1785,7 @@ en: dimensions_caption: "How many numbers each vector has. Leave blank to use whatever the server returns." dimensions_caption_probed: "The server returned %{dimensions}-dimension vectors for this model. Leave blank to use whatever it returns at index time." input_prefix_caption: "Prepended to each document before it is indexed. Some models expect one, for example \"passage: \" including the trailing space." + model_caption_embedding: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." query_prefix_caption: "Prepended to each search query. Some models expect one, for example \"query: \" including the trailing space." index: blank_description: "Connect OpenProject to an LLM server first. Models can be assigned once the server reports which ones it offers." @@ -1794,8 +1795,7 @@ en: inherit_without_default: "Use the default (none set)" locked: "%{model} is in use by indexed data and cannot be changed here. Re-index to switch models." locked_values_heading: "Values fixed by the existing index" - option_unknown: "%{model} — not verified" - option_unsupported: "%{model} — no %{capability} support" + option_unqualified: "%{model} (no longer eligible for this feature)" update: model_incapable: "The model for %{feature} has been saved, but the server just reported that it does not support %{capability}. Pick a different model, or assert the capability on the model if you know better." success: "The model for %{feature} has been saved." diff --git a/spec/models/llm_model_spec.rb b/spec/models/llm_model_spec.rb index 7b0aeb5bd037..1416ffc710f6 100644 --- a/spec/models/llm_model_spec.rb +++ b/spec/models/llm_model_spec.rb @@ -87,6 +87,18 @@ expect(options.map(&:model_id)).to include("qwen3.6-27b") end + it "still offers it to an embedding feature that is bound to it" do + connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", + state: "supported", source: "admin", checked_at: Time.current) + connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "qwen3.6-27b") + + options = LlmConnections::SelectableModelsQuery + .new(connection, OpenProject::Llm::Features[:semantic_search]) + .call + + expect(options.map(&:model_id)).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". diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index e32dca579b54..2b060c15b29c 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -35,6 +35,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_models(feature_key) + items = page.find("form[action='#{llm_feature_binding_path(feature_key)}'] opce-autocompleter")["data-items"] + + JSON.parse(items).pluck("id").compact_blank + end + describe "GET /admin/llm_feature_bindings" do before { login_as admin } @@ -64,21 +72,33 @@ expect(response.body).to include(llm_models_path) end - # Hiding an unusable model is the one thing that produces an unanswerable - # support question, so it stays listed and says why it cannot be chosen. - it "offers a model with no verdict, marked as unverified" do + # An unconfirmed capability is not a capability: offering such a model + # invites a choice that fails much later, at index time. + it "offers an embedding feature only models known to create embeddings" do + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + + get llm_feature_bindings_path + + expect(offered_models(:semantic_search)).to contain_exactly("bge-m3") + expect(offered_models(:description_assistant)).to contain_exactly("qwen3.6-27b") + end + + it "says so, and where to read up on model types" do get llm_feature_bindings_path - expect(response.body).to include("qwen3.6-27b — not verified") + expect(response.body).to include("Only models known to create embeddings are offered") + expect(response.body).to include("huggingface.co/blog/getting-started-with-embeddings") end - it "disables a model known not to support a required capability" do - connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", - state: "unsupported", source: "probe", checked_at: Time.current) + # Otherwise opening the page and saving it would blank a working binding. + it "keeps the bound model listed once it no longer qualifies" do + connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "qwen3.6-27b") get llm_feature_bindings_path - expect(response.body).to include("qwen3.6-27b — no Embeddings support") + expect(offered_models(:semantic_search)).to include("qwen3.6-27b") + expect(response.body).to include("no longer eligible for this feature") end end end From c8b103444aadeea6688b9e8c8513ba107b703200 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 12:19:38 +0200 Subject: [PATCH 04/10] [AI-3] Preset embedding prefixes from the feature registration A feature that embeds now declares its document and query prefixes where it registers, defaulting to its own key, and the binding form starts out with them filled in. That was the request from the UX review with Tom: an administrator should not have to invent a prefix to configure semantic search. A prefix an administrator deliberately empties is still stored as empty, and a chat feature that passes a prefix is refused at registration time, since nothing would ever read it. --- .../feature_binding_component.rb | 4 +- .../admin/llm_feature_bindings/index.html.erb | 8 ++- config/locales/en.yml | 4 +- lib_static/open_project/llm/features.rb | 28 +++++++- spec/lib/open_project/llm/features_spec.rb | 69 +++++++++++++++++++ .../admin/llm_feature_bindings_spec.rb | 15 ++++ spec/requests/admin/llm_models_spec.rb | 1 + 7 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 spec/lib/open_project/llm/features_spec.rb diff --git a/app/components/llm_connections/feature_binding_component.rb b/app/components/llm_connections/feature_binding_component.rb index 9d5d16e5a555..0d2e2db935ce 100644 --- a/app/components/llm_connections/feature_binding_component.rb +++ b/app/components/llm_connections/feature_binding_component.rb @@ -44,7 +44,9 @@ def initialize(feature:, connection:, binding: nil) # The record the select binds to. A feature without a stored binding still # needs one so the form has a model_id to read. def form_model - binding || connection.feature_bindings.new(feature_key: feature.key.to_s) + binding || connection.feature_bindings.new(feature_key: feature.key.to_s, + input_prefix: feature.input_prefix, + query_prefix: feature.query_prefix) end # Not named +options+: ApplicationComponent already owns that name and diff --git a/app/views/admin/llm_feature_bindings/index.html.erb b/app/views/admin/llm_feature_bindings/index.html.erb index 499a5cb4b9c2..8b75c912969c 100644 --- a/app/views/admin/llm_feature_bindings/index.html.erb +++ b/app/views/admin/llm_feature_bindings/index.html.erb @@ -33,9 +33,11 @@ See COPYRIGHT and LICENSE files for more details. render(Primer::OpenProject::PageHeader.new) do |header| header.with_title { t("menus.admin.llm_feature_bindings") } header.with_description do - link_translate("admin.llm_feature_bindings.index.description", - links: { models_url: llm_models_path }, - external: false) + link_translate( + "admin.llm_feature_bindings.index.description", + links: { models_url: llm_models_path }, + external: false + ) end header.with_breadcrumbs( [{ href: admin_index_path, text: t(:label_administration) }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 973b0c6c47f3..5c0334b3f96f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1784,9 +1784,9 @@ en: form: dimensions_caption: "How many numbers each vector has. Leave blank to use whatever the server returns." dimensions_caption_probed: "The server returned %{dimensions}-dimension vectors for this model. Leave blank to use whatever it returns at index time." - input_prefix_caption: "Prepended to each document before it is indexed. Some models expect one, for example \"passage: \" including the trailing space." + input_prefix_caption: "Prepended to each document before it is indexed. Preset for this feature; some models expect their own, for example \"passage: \" including the trailing space." model_caption_embedding: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." - query_prefix_caption: "Prepended to each search query. Some models expect one, for example \"query: \" including the trailing space." + query_prefix_caption: "Prepended to each search query. Preset for this feature; some models expect their own, for example \"query: \" including the trailing space." index: blank_description: "Connect OpenProject to an LLM server first. Models can be assigned once the server reports which ones it offers." blank_title: "No LLM server configured" diff --git a/lib_static/open_project/llm/features.rb b/lib_static/open_project/llm/features.rb index 5165d24ca2e8..d694a3520b0b 100644 --- a/lib_static/open_project/llm/features.rb +++ b/lib_static/open_project/llm/features.rb @@ -37,7 +37,8 @@ class UnknownFeature < StandardError; end # Features declare the capabilities they need so the administration UI can # tell an administrator which models are usable for which job, and so a # feature never silently runs against a model that cannot serve it. - Feature = Data.define(:key, :kind, :requires, :prefers, :overridable, :pinned, :available, :i18n_scope) do + Feature = Data.define(:key, :kind, :requires, :prefers, :overridable, :pinned, :available, :i18n_scope, + :input_prefix, :query_prefix) do def available? = available.call def chat? = kind == :chat @@ -82,14 +83,19 @@ def register(key, overridable: false, pinned: false, available: -> { true }, - i18n_scope: nil) + i18n_scope: nil, + input_prefix: nil, + query_prefix: nil) key = key.to_sym validate!(key, kind, requires + prefers) + validate_prefixes!(key, kind, [input_prefix, query_prefix]) all[key] = Feature.new(key:, kind:, requires: requires.map(&:to_sym).freeze, prefers: prefers.map(&:to_sym).freeze, overridable:, pinned:, available:, - i18n_scope: i18n_scope || "llm.features.#{key}") + i18n_scope: i18n_scope || "llm.features.#{key}", + input_prefix: prefix_for(kind, key, input_prefix), + query_prefix: prefix_for(kind, key, query_prefix)) end def all = @all ||= {} @@ -106,6 +112,22 @@ def available = all.values.select(&:available?) def for_kind(kind) = available.select { |feature| feature.kind == kind } + # Each embedding feature indexes its own documents, so its prefixes are + # derived from the key unless the feature names better ones. Administrators + # then never have to type them, while a model that expects its own, such as + # "passage: ", can still be accommodated on the binding. + def prefix_for(kind, key, given) + return unless kind == :embedding + + given || "#{key}_" + end + + def validate_prefixes!(key, kind, prefixes) + return if kind == :embedding || prefixes.none? + + raise ArgumentError, "prefixes are not valid for the chat feature #{key}" + end + def validate!(key, kind, capabilities) raise ArgumentError, "unknown kind #{kind.inspect}" unless KINDS.include?(kind) raise ArgumentError, "LLM feature #{key} is already registered" if all.key?(key) diff --git a/spec/lib/open_project/llm/features_spec.rb b/spec/lib/open_project/llm/features_spec.rb new file mode 100644 index 000000000000..da8f54f1fc55 --- /dev/null +++ b/spec/lib/open_project/llm/features_spec.rb @@ -0,0 +1,69 @@ +# 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 OpenProject::Llm::Features do + let(:key) { :spec_only_feature } + + after { described_class.all.delete(key) } + + describe "prefixes" do + it "derives them from the key of an embedding feature" do + described_class.register(key, kind: :embedding, requires: %i[embeddings]) + + expect(described_class[key].input_prefix).to eq("spec_only_feature_") + expect(described_class[key].query_prefix).to eq("spec_only_feature_") + end + + it "keeps the ones a feature names itself" do + described_class.register(key, kind: :embedding, input_prefix: "passage: ", query_prefix: "query: ") + + expect(described_class[key].input_prefix).to eq("passage: ") + expect(described_class[key].query_prefix).to eq("query: ") + end + + it "leaves a chat feature without any" do + described_class.register(key, kind: :chat) + + expect(described_class[key].input_prefix).to be_nil + expect(described_class[key].query_prefix).to be_nil + end + + it "refuses one on a chat feature" do + expect { described_class.register(key, kind: :chat, input_prefix: "passage: ") } + .to raise_error(ArgumentError, /chat feature/) + end + + it "presets the ones semantic search is registered with" do + expect(described_class[:semantic_search].input_prefix).to eq("semantic_search_") + end + end +end diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index 2b060c15b29c..3917c9a962b3 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -179,6 +179,21 @@ def offered_models(feature_key) expect(binding.query_prefix).to eq("query: ") end + # Typing a prefix by hand is a chore nobody should have to get right, so the + # registration supplies one and an untouched save stores it. + it "prefills the prefixes from the feature registration" do + get llm_feature_bindings_path + + expect(response.body).to include('value="semantic_search_"') + end + + it "stores a cleared prefix as empty rather than restoring the default" do + patch llm_feature_binding_path(:semantic_search), + params: { llm_feature_binding: { model_id: "bge-m3", input_prefix: "" } } + + expect(connection.feature_bindings.find_by(feature_key: "semantic_search").input_prefix).to eq("") + end + it "rejects a dimension count that is not a positive integer" do patch llm_feature_binding_path(:semantic_search), params: { llm_feature_binding: { model_id: "bge-m3", dimensions: "0" } } diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index da97cc684a8f..d67701e99d70 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -180,6 +180,7 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) get llm_models_path expect(offered_default_models(:default_embedding_model_id)).to contain_exactly("bge-m3") + expect(response.body).to include("huggingface.co/blog/getting-started-with-embeddings") end # An unconfirmed capability is not a capability: offering such a model From 3bcf0305cb162f6afa2c75e044cabdb4c7be9af1 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 13:10:26 +0200 Subject: [PATCH 05/10] [AI-3] Show the feature configuration as a tab of the LLM settings page The AI section of the administration keeps a single entry, LLM settings. Feature configuration joins the connection and the AI models as its third tab, visible only while the connection is switched on, so the sidebar no longer lists three siblings that only make sense together. Admin::LlmFeatureBindingsController highlights the LLM settings entry and sends a disabled or missing connection back to the settings page, which makes the no-connection blankslate unreachable; it goes with its keys. --- .../default_models_component.rb | 2 +- .../admin/llm_feature_bindings_controller.rb | 7 +++- app/helpers/llm_connections_helper.rb | 3 +- .../admin/llm_feature_bindings/index.html.erb | 34 +++++++------------ config/initializers/llm_features.rb | 2 +- config/initializers/menus.rb | 6 ---- config/locales/en.yml | 4 +-- spec/features/admin/llm_connection_spec.rb | 9 +++-- spec/requests/admin/llm_connections_spec.rb | 5 +-- .../admin/llm_feature_bindings_spec.rb | 25 +++++++++++--- .../env_data/llm_connection_seeder_spec.rb | 2 +- 11 files changed, 54 insertions(+), 45 deletions(-) diff --git a/app/components/llm_connections/default_models_component.rb b/app/components/llm_connections/default_models_component.rb index 33762c03cf74..4b7a56be7a7d 100644 --- a/app/components/llm_connections/default_models_component.rb +++ b/app/components/llm_connections/default_models_component.rb @@ -29,7 +29,7 @@ #++ module LlmConnections - # The "Default models" section of the LLMs page. + # The "Default models" section of the LLMs tab. class DefaultModelsComponent < ApplicationComponent include ApplicationHelper include OpPrimer::ComponentHelpers diff --git a/app/controllers/admin/llm_feature_bindings_controller.rb b/app/controllers/admin/llm_feature_bindings_controller.rb index c531695799ee..0a9fad30a78e 100644 --- a/app/controllers/admin/llm_feature_bindings_controller.rb +++ b/app/controllers/admin/llm_feature_bindings_controller.rb @@ -32,11 +32,12 @@ module Admin # Assigns a model to each registered AI feature. class LlmFeatureBindingsController < ApplicationController layout "admin" - menu_item :llm_feature_bindings + menu_item :llm_connection before_action :require_feature before_action :require_admin before_action :set_connection + before_action :require_enabled_connection def index @features = OpenProject::Llm::Features.available @@ -58,6 +59,10 @@ def set_connection @connection = LlmConnection.instance end + def require_enabled_connection + redirect_to llm_connection_path, status: :see_other unless @connection.enabled? + end + # The flag gates the endpoints, not only the menu entry: an unfinished page # must not accept writes just because somebody knows the URL. def require_feature diff --git a/app/helpers/llm_connections_helper.rb b/app/helpers/llm_connections_helper.rb index 06d29c99ff95..e7b40e05ccd5 100644 --- a/app/helpers/llm_connections_helper.rb +++ b/app/helpers/llm_connections_helper.rb @@ -37,7 +37,8 @@ def llm_settings_tabs(_connection) [ { name: "connection", path: llm_connection_path, label: t("admin.llm_connections.tabs.connection") }, - { name: "models", path: llm_models_path, label: t("admin.llm_connections.tabs.models") } + { name: "models", path: llm_models_path, label: t("admin.llm_connections.tabs.models") }, + { name: "features", path: llm_feature_bindings_path, label: t("admin.llm_connections.tabs.features") } ] end end diff --git a/app/views/admin/llm_feature_bindings/index.html.erb b/app/views/admin/llm_feature_bindings/index.html.erb index 8b75c912969c..2b330a63570d 100644 --- a/app/views/admin/llm_feature_bindings/index.html.erb +++ b/app/views/admin/llm_feature_bindings/index.html.erb @@ -27,11 +27,11 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<% html_title t(:label_administration), t("menus.admin.llm_feature_bindings") %> +<% html_title t(:label_administration), t("menus.admin.llm_connection"), t("admin.llm_connections.tabs.features") %> <%= render(Primer::OpenProject::PageHeader.new) do |header| - header.with_title { t("menus.admin.llm_feature_bindings") } + header.with_title { t("menus.admin.llm_connection") } header.with_description do link_translate( "admin.llm_feature_bindings.index.description", @@ -42,28 +42,18 @@ See COPYRIGHT and LICENSE files for more details. header.with_breadcrumbs( [{ href: admin_index_path, text: t(:label_administration) }, { href: mcp_configurations_path, text: t("menus.admin.ai") }, - t("menus.admin.llm_feature_bindings")] + t("menus.admin.llm_connection")] ) + render_tab_header_nav(header, llm_settings_tabs(@connection), test_selector: "llm-settings--tabs") end %> -<% if !@connection.configured? %> - <%= - render(Primer::Beta::Blankslate.new(border: true)) do |component| - component.with_visual_icon(icon: :sparkle) - component.with_heading(tag: :h2) { t(".blank_title") } - component.with_description { t(".blank_description") } - component.with_primary_action(href: llm_connection_path) { t("menus.admin.llm_connection") } - end - %> -<% else %> - <% @features.each do |feature| %> - <%= render( - LlmConnections::FeatureBindingComponent.new( - feature:, - connection: @connection, - binding: @bindings[feature.key.to_s] - ) - ) %> - <% end %> +<% @features.each do |feature| %> + <%= render( + LlmConnections::FeatureBindingComponent.new( + feature:, + connection: @connection, + binding: @bindings[feature.key.to_s] + ) + ) %> <% end %> diff --git a/config/initializers/llm_features.rb b/config/initializers/llm_features.rb index 7714446f1ff6..5e940a13f448 100644 --- a/config/initializers/llm_features.rb +++ b/config/initializers/llm_features.rb @@ -33,7 +33,7 @@ # Features that send requests to the configured LLM server. # # Add a feature here (or from a module engine initializer) so that -# administrators can assign it a model on the "Feature configuration" page. +# administrators can assign it a model on the "Feature configuration" tab. # The description assistant rewrites work package text on explicit user action. # Plain chat completions only: no tools, no JSON mode, no streaming. Individual diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index 2872ac21a09a..bed2ac6b42cc 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -510,12 +510,6 @@ caption: I18n.t("menus.admin.llm_connection"), parent: :ai - menu.push :llm_feature_bindings, - { controller: "/admin/llm_feature_bindings", action: :index }, - if: ->(_) { User.current.admin? && OpenProject::FeatureDecisions.llm_connection_active? }, - caption: I18n.t("menus.admin.llm_feature_bindings"), - parent: :ai - menu.push :mcp_configurations, { controller: "/admin/mcp_configurations", action: :index }, if: ->(_) { User.current.admin? }, diff --git a/config/locales/en.yml b/config/locales/en.yml index 5c0334b3f96f..2a7ec584ace4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1773,6 +1773,7 @@ en: description_enabled: "Once connected, review the models offered by the server on the [LLMs](models_url) tab." tabs: connection: "Connection" + features: "Feature configuration" models: "LLMs" update: disabled: "Saved. LLM features are switched off for this instance." @@ -1788,8 +1789,6 @@ en: model_caption_embedding: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." query_prefix_caption: "Prepended to each search query. Preset for this feature; some models expect their own, for example \"query: \" including the trailing space." index: - blank_description: "Connect OpenProject to an LLM server first. Models can be assigned once the server reports which ones it offers." - blank_title: "No LLM server configured" description: "Choose which model each AI feature uses. Features without a choice use the default models set on the [LLMs](models_url) tab." inherit_with_default: "Use the default (%{model})" inherit_without_default: "Use the default (none set)" @@ -4662,7 +4661,6 @@ en: ai: "Artificial Intelligence (AI)" api_and_webhooks: "API and webhooks" llm_connection: "LLM settings" - llm_feature_bindings: "Feature configuration" mail_notification: "Email notifications" mails_and_notifications: "Emails and notifications" mcp_configurations: "Model Context Protocol (MCP)" diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index a12b223fa46c..f48283610d91 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -83,7 +83,7 @@ def choose_action(item) expect(page).to have_field("Host URL") end - it "offers the models tab only once the connection is enabled" do + it "offers the models and feature tabs only once the connection is enabled" do mock_llm_models_response(base_url) visit llm_connection_path @@ -99,6 +99,10 @@ def choose_action(item) within_test_selector("llm-settings--tabs") { click_on "LLMs" } expect(page).to have_current_path(llm_models_path) + + within_test_selector("llm-settings--tabs") { click_on "Feature configuration" } + + expect(page).to have_current_path(llm_feature_bindings_path) end it "describes the server the selected API format expects" do @@ -234,7 +238,7 @@ def choose_action(item) end end - describe "the Feature configuration page" do + describe "the Feature configuration tab" do let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } before { mock_llm_embeddings_response(base_url) } @@ -242,6 +246,7 @@ def choose_action(item) it "offers the vector settings only for features that embed" do visit llm_feature_bindings_path + expect(page).to have_test_selector("llm-settings--tabs") expect(page).to have_test_selector("llm-feature-binding--dimensions-semantic_search") expect(page).to have_no_test_selector("llm-feature-binding--dimensions-description_assistant") expect(page).to be_axe_clean.within("#content") diff --git a/spec/requests/admin/llm_connections_spec.rb b/spec/requests/admin/llm_connections_spec.rb index 8a80185d15d6..7e1c6072662d 100644 --- a/spec/requests/admin/llm_connections_spec.rb +++ b/spec/requests/admin/llm_connections_spec.rb @@ -103,7 +103,7 @@ expect(page).to have_css("a[href='#{llm_models_path}']", text: "LLMs") end - it "offers the LLMs tab once the features are on", + it "offers the LLMs and Feature configuration tabs once the features are on", with_settings: { llm_features_enabled: true } do create(:llm_connection, base_url:) @@ -111,6 +111,7 @@ expect(response.body).to include("llm-settings--tabs") expect(response.body).to include(llm_models_path) + expect(response.body).to include(llm_feature_bindings_path) end context "when an API key is stored" do @@ -421,7 +422,7 @@ before { login_as admin } - it "are chosen on the LLMs page, not here" do + it "are chosen on the LLMs tab, not here" do patch llm_connection_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b", default_embedding_model_id: "bge-m3" } } diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index 3917c9a962b3..6ccaf7d2eb6a 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -46,15 +46,22 @@ def offered_models(feature_key) describe "GET /admin/llm_feature_bindings" do before { login_as admin } - it "prompts to configure a connection when there is none" do + it "sends the administrator to the settings while no connection is stored" do get llm_feature_bindings_path - expect(response).to have_http_status(:ok) - expect(response.body).to include("No LLM server configured") + expect(response).to redirect_to(llm_connection_path) + end + + it "sends the administrator to the settings while the connection is disabled" do + create(:llm_connection, :with_models, base_url:) + + get llm_feature_bindings_path + + expect(response).to redirect_to(llm_connection_path) end context "with a configured connection" do - let!(:connection) { create(:llm_connection, :with_models, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } it "lists every registered feature" do get llm_feature_bindings_path @@ -65,6 +72,14 @@ def offered_models(feature_key) expect(response.body).to include("Semantic search") end + it "offers the tabs of the LLM settings page" do + get llm_feature_bindings_path + + expect(page).to have_css("[data-test-selector='llm-settings--tabs'] a[href='#{llm_connection_path}']") + expect(page).to have_css("[data-test-selector='llm-settings--tabs'] a[href='#{llm_models_path}']") + expect(page).to have_css("[data-test-selector='llm-settings--tabs'] a[href='#{llm_feature_bindings_path}']") + end + it "points at the page where the default models are chosen" do get llm_feature_bindings_path @@ -104,7 +119,7 @@ def offered_models(feature_key) end describe "PATCH /admin/llm_feature_bindings/:id" do - let!(:connection) { create(:llm_connection, :with_models, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } before { login_as admin } diff --git a/spec/seeders/env_data/llm_connection_seeder_spec.rb b/spec/seeders/env_data/llm_connection_seeder_spec.rb index 1495a8c5affa..9ba3571773f1 100644 --- a/spec/seeders/env_data/llm_connection_seeder_spec.rb +++ b/spec/seeders/env_data/llm_connection_seeder_spec.rb @@ -64,7 +64,7 @@ # The seeder runs on every container start, so a refresh here would repeatedly # overwrite a list an administrator has curated. The stale warning on the AI - # models page asks for the refresh instead. + # models tab asks for the refresh instead. context "when the environment moves a stored catalogue to another host", with_settings: { llm_connection: { "base_url" => "https://other.example.com/v1", "api_key" => "sk-from-env" } } do From aca26ef920b445f6f33112b2f702c09507a48634 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 14:12:11 +0200 Subject: [PATCH 06/10] [AI-3] Refuse an embedding default only when it is known not to embed The contract judged the default embedding model by LlmModel#embedding?, which is false both for a model the server ruled out and for one nothing has probed yet. An unknown verdict is the normal state of a self-hosted server, so provisioning from the environment failed as soon as the catalogue carried a row for the configured model, while the identical configuration succeeded on an empty catalogue. Only a blocking verdict is refused now. The picker still offers confirmed models only, so nothing about the administrator's choice changes. --- .../llm_connections/base_contract.rb | 9 ++--- config/locales/en.yml | 2 +- spec/requests/admin/llm_models_spec.rb | 13 ++++++- .../env_data/llm_connection_seeder_spec.rb | 36 +++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index c32f3cfd46e0..0bb8088e1447 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -65,9 +65,10 @@ def not_configured_from_env private - # Only a model known to embed can serve the embedding default, the same rule - # the picker offers by. A model the connection does not know at all is left - # to default_models_offered_by_server. + # Only a model the server has ruled out is refused. "We could not tell" is the + # normal state on a self-hosted server, and an environment-provisioned default + # would otherwise fail on a catalogue row nothing has probed yet. The picker + # still offers confirmed models only. def default_embedding_model_can_embed model_id = model.default_embedding_model_id return if model_id.blank? @@ -75,7 +76,7 @@ def default_embedding_model_can_embed llm_model = model.models.find_by(external_id: model_id) - errors.add(:default_embedding_model_id, :cannot_embed) if llm_model && !llm_model.embedding? + errors.add(:default_embedding_model_id, :cannot_embed) if llm_model&.verdict_for(:embeddings)&.blocking? end # A model the server identifies as an embedding model is not a chat candidate. diff --git a/config/locales/en.yml b/config/locales/en.yml index 2a7ec584ace4..3590c580f474 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -714,7 +714,7 @@ en: 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: "is not known to create embeddings." + cannot_embed: "is known not to 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." diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index d67701e99d70..7c7a064da551 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -761,13 +761,24 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size expect(connection.reload.default_embedding_model_id).to eq("bge-m3") end - it "refuses a model that is not known to embed" do + it "refuses a model the server has ruled out" do + connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", + state: "unsupported", source: "probe", checked_at: Time.current) + patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "qwen3.6-27b" } } expect(connection.reload.default_embedding_model_id).to be_nil expect(flash[:error]).to be_present end + # Nothing has probed the row: refusing here would break provisioning from the + # environment, where the same configuration passes on an empty catalogue. + it "accepts a model nothing has ruled out" do + patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "qwen3.6-27b" } } + + expect(connection.reload.default_embedding_model_id).to eq("qwen3.6-27b") + end + it "leaves the server settings alone" do patch defaults_llm_models_path, params: { llm_connection: { default_chat_model_id: chat_model.id, base_url: "https://elsewhere.test/v1" } } diff --git a/spec/seeders/env_data/llm_connection_seeder_spec.rb b/spec/seeders/env_data/llm_connection_seeder_spec.rb index 9ba3571773f1..9e76c6c50b0d 100644 --- a/spec/seeders/env_data/llm_connection_seeder_spec.rb +++ b/spec/seeders/env_data/llm_connection_seeder_spec.rb @@ -82,6 +82,42 @@ end end + # An unknown verdict is the normal state of a self-hosted server: nothing has + # probed the row yet. Provisioning must not fail on it, or the identical + # configuration would seed on a fresh installation and raise on the next start. + context "with an embedding default nothing has probed", with_settings: { + llm_connection: { + "base_url" => "https://example.com/v1", + "default_embedding_model" => "bge-m3" + } + } do + before { create(:llm_connection, :with_models, base_url: "https://example.com/v1") } + + it "provisions the default" do + seed + + expect(LlmConnection.first.default_embedding_model_id).to eq("bge-m3") + end + end + + context "with an embedding default the server has ruled out", with_settings: { + llm_connection: { + "base_url" => "https://example.com/v1", + "default_embedding_model" => "bge-m3" + } + } do + before do + create(:llm_connection, :with_models, base_url: "https://example.com/v1") + .capability_verdicts + .create!(model_id: "bge-m3", capability: "embeddings", + state: "unsupported", source: "probe", checked_at: Time.current) + end + + it "refuses to provision it" do + expect { seed }.to raise_error(/create embeddings/) + end + end + # The environment is the source of truth while the form is read-only under it, # so a value removed from the environment must not linger in the database. context "when a previously set key is removed from the environment", with_settings: { From 867e5963585176d674ea767829232e4e46ca6a94 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 15:30:14 +0200 Subject: [PATCH 07/10] [AI-3] Tell a switched-off default apart from an unqualified one The embedding picker read capability off the list of selectable models, so switching off a model the server confirmed relabelled it as not known to create embeddings, and where it was the only one the caption asked the administrator to set a type that was already right. Whether a model embeds is a fact about the server; whether it is offered is the administrator's choice. LlmConnection#embedding_capable_model_ids now answers the first, and the form uses it for the label and the caption. --- .../llm_connections/default_models_form.rb | 17 +++++++++-- app/models/llm_connection.rb | 13 ++++++-- config/locales/en.yml | 1 + spec/requests/admin/llm_models_spec.rb | 30 +++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/app/forms/llm_connections/default_models_form.rb b/app/forms/llm_connections/default_models_form.rb index a132427c25c5..6bc4d6a841f3 100644 --- a/app/forms/llm_connections/default_models_form.rb +++ b/app/forms/llm_connections/default_models_form.rb @@ -110,16 +110,29 @@ def embedding_models @embedding_models ||= model.embedding_models end + def embedding_capable_models + @embedding_capable_models ||= model.embedding_capable_models + end + + # Switched off and unqualified are different problems with different remedies, + # so a stored default that embeds but was hidden says so rather than claiming + # the server never confirmed it. def embedding_option_label(llm_model) return llm_model.name if embedding_models.include?(llm_model) - I18n.t("admin.llm_models.defaults.embedding_option_unqualified", model: llm_model.name) + key = if embedding_capable_models.include?(llm_model) + "embedding_option_deactivated" + else + "embedding_option_unqualified" + end + + I18n.t("admin.llm_models.defaults.#{key}", model: llm_model.name) end # Says how to make a model eligible when none is, rather than leaving an # empty picker with no explanation. def embedding_caption - return I18n.t("admin.llm_models.defaults.embedding_none") if embedding_models.empty? + return I18n.t("admin.llm_models.defaults.embedding_none") if embedding_capable_models.empty? link_translate("admin.llm_models.defaults.embedding_caption", links: { docs_url: %i[embeddings_explanation] }, diff --git a/app/models/llm_connection.rb b/app/models/llm_connection.rb index 41425944e38c..af22d5163ea1 100644 --- a/app/models/llm_connection.rb +++ b/app/models/llm_connection.rb @@ -126,12 +126,19 @@ def embedding_models selectable_models.select(&:embedding?) end - def embedding_model_ids - embedding = capability_verdicts.for_capability(:embeddings).where(state: "supported").pluck(:model_id) + # Every model the server is known to embed with, including ones an + # administrator has switched off: whether a model can embed is a fact about + # the server, not about what a picker currently offers. + def embedding_capable_models + models.active.by_identifier.select(&:embedding?) + end - selectable_model_ids & embedding + def embedding_capable_model_ids + capability_verdicts.for_capability(:embeddings).where(state: "supported").pluck(:model_id) end + def embedding_model_ids = selectable_model_ids & embedding_capable_model_ids + def chat_model_ids = selectable_model_ids - embedding_model_ids def server_flavour diff --git a/config/locales/en.yml b/config/locales/en.yml index 3590c580f474..a347f1ec973a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1806,6 +1806,7 @@ en: description: "Used by AI features that do not choose a model themselves." embedding_caption: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." embedding_none: "No model is known to create embeddings. If you know one does, open it in the list below and set its type to Embedding model." + embedding_option_deactivated: "%{model} (switched off)" embedding_option_unqualified: "%{model} (not known to create embeddings)" heading: "Default models" success: "The default models have been saved." diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index 7c7a064da551..de626103f2a4 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -193,6 +193,36 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) expect(response.body).to include("set its type to Embedding model") end + # Switched off is a different problem from unqualified: the model does embed, + # an administrator simply hid it, and saying otherwise sends them to the + # model form to fix a type that is already right. + it "tells a switched-off embedding default apart from an unqualified one" do + connection = create(:llm_connection, :with_models, :enabled, base_url:) + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + connection.update_column(:default_embedding_model_id, "bge-m3") + connection.models.find_by(external_id: "bge-m3").update!(deactivated_at: Time.current) + + get llm_models_path + + expect(offered_default_models(:default_embedding_model_id)).to include("bge-m3") + expect(response.body).to include("switched off") + expect(response.body).not_to include("not known to create embeddings") + end + + # The remedy the caption names is only the right one while nothing embeds. + it "keeps the documentation caption while a known embedding model is switched off" do + connection = create(:llm_connection, :with_models, :enabled, base_url:) + connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", + state: "supported", source: "probe", checked_at: Time.current) + connection.models.find_by(external_id: "bge-m3").update!(deactivated_at: Time.current) + + get llm_models_path + + expect(response.body).to include("huggingface.co/blog/getting-started-with-embeddings") + expect(response.body).not_to include("set its type to Embedding model") + end + # Otherwise a save would silently blank a working configuration. it "keeps the stored embedding default listed, flagged, once it is ruled out" do connection = create(:llm_connection, :with_models, :enabled, base_url:) From 2a76b55f3d95739e1c9cc59cb05207be322c0665 Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 16:26:54 +0200 Subject: [PATCH 08/10] [AI-3] Say why the feature tab sends you back Opening the feature configuration URL while LLM features are switched off bounced the administrator back to the settings page in silence. It now carries the same notice the LLMs tab uses. --- app/controllers/admin/llm_feature_bindings_controller.rb | 5 ++++- spec/requests/admin/llm_feature_bindings_spec.rb | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/llm_feature_bindings_controller.rb b/app/controllers/admin/llm_feature_bindings_controller.rb index 0a9fad30a78e..90055f0edd25 100644 --- a/app/controllers/admin/llm_feature_bindings_controller.rb +++ b/app/controllers/admin/llm_feature_bindings_controller.rb @@ -60,7 +60,10 @@ def set_connection end def require_enabled_connection - redirect_to llm_connection_path, status: :see_other unless @connection.enabled? + return if @connection.enabled? + + flash[:notice] = t("admin.llm_connections.disabled_notice") + redirect_to llm_connection_path, status: :see_other end # The flag gates the endpoints, not only the menu entry: an unfinished page diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index 6ccaf7d2eb6a..e7c71bac9cce 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -58,6 +58,7 @@ def offered_models(feature_key) get llm_feature_bindings_path expect(response).to redirect_to(llm_connection_path) + expect(flash[:notice]).to eq(I18n.t("admin.llm_connections.disabled_notice")) end context "with a configured connection" do From 54f01060d669211588596d6fb90170926a9bac8a Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 17:30:08 +0200 Subject: [PATCH 09/10] [AI-3] Drop the document and query prefix settings Nothing read them: Llm::Runtime::Resolution#embed passes only the model and the dimensions, while the caption promised that the document prefix is prepended to each document before it is indexed. An administrator was configuring fields that did nothing, and the prefix a model expects is a property of the embedder, not of an OpenProject feature. The embedding dimensions stay: the health check reads them and a stored index depends on them. The columns come off in a new migration rather than by editing 20260811140100_create_llm_feature_bindings.rb, whose commit is already on GitHub. --- .../feature_binding_component.rb | 10 +---- .../admin/llm_feature_bindings_controller.rb | 13 ++----- .../llm_connections/feature_binding_form.rb | 20 +--------- app/models/llm_feature_binding.rb | 7 +--- config/locales/en.yml | 10 ----- ...move_prefixes_from_llm_feature_bindings.rb | 38 +++++++++++++++++++ lib_static/open_project/llm/features.rb | 28 ++------------ spec/lib/open_project/llm/features_spec.rb | 35 +++++++---------- .../admin/llm_feature_bindings_spec.rb | 32 +++++----------- 9 files changed, 71 insertions(+), 122 deletions(-) create mode 100644 db/migrate/20260908090000_remove_prefixes_from_llm_feature_bindings.rb diff --git a/app/components/llm_connections/feature_binding_component.rb b/app/components/llm_connections/feature_binding_component.rb index 0d2e2db935ce..076c748bdc6a 100644 --- a/app/components/llm_connections/feature_binding_component.rb +++ b/app/components/llm_connections/feature_binding_component.rb @@ -44,9 +44,7 @@ def initialize(feature:, connection:, binding: nil) # The record the select binds to. A feature without a stored binding still # needs one so the form has a model_id to read. def form_model - binding || connection.feature_bindings.new(feature_key: feature.key.to_s, - input_prefix: feature.input_prefix, - query_prefix: feature.query_prefix) + binding || connection.feature_bindings.new(feature_key: feature.key.to_s) end # Not named +options+: ApplicationComponent already owns that name and @@ -78,14 +76,10 @@ def probed_dimensions connection.capability_verdicts.for_model(model_id).for_capability(:embeddings).first&.dimensions end - # Quoted so a trailing space -- load-bearing for the E5 and BGE families -- - # is visible rather than invisible. def locked_values [ [LlmFeatureBinding.human_attribute_name(:model_id), binding.model_id], - [LlmFeatureBinding.human_attribute_name(:dimensions), binding.dimensions || "—"], - [LlmFeatureBinding.human_attribute_name(:input_prefix), binding.input_prefix.to_s.inspect], - [LlmFeatureBinding.human_attribute_name(:query_prefix), binding.query_prefix.to_s.inspect] + [LlmFeatureBinding.human_attribute_name(:dimensions), binding.dimensions || "—"] ] end diff --git a/app/controllers/admin/llm_feature_bindings_controller.rb b/app/controllers/admin/llm_feature_bindings_controller.rb index 90055f0edd25..9be84933659d 100644 --- a/app/controllers/admin/llm_feature_bindings_controller.rb +++ b/app/controllers/admin/llm_feature_bindings_controller.rb @@ -108,22 +108,15 @@ def build_binding(feature) binding = binding_for(feature) binding.model_id = params.dig(:llm_feature_binding, :model_id).presence - # Only ever accepted for the kind of feature they describe; the model - # rejects them elsewhere, and they are not read at all for a chat feature. + # Only ever accepted for the kind of feature it describes; the model + # rejects it elsewhere, and it is not read at all for a chat feature. assign_embedding_settings(binding) if feature.embedding? binding end - # The prefixes are stored exactly as typed. The trailing space in "passage: " - # is load-bearing for the E5 and BGE families, so stripping would silently - # degrade retrieval. def assign_embedding_settings(binding) - settings = params.fetch(:llm_feature_binding, {}) - - binding.dimensions = settings[:dimensions].presence - binding.input_prefix = settings[:input_prefix] - binding.query_prefix = settings[:query_prefix] + binding.dimensions = params.dig(:llm_feature_binding, :dimensions).presence end # The verdict that actually matters is the one for the model an administrator diff --git a/app/forms/llm_connections/feature_binding_form.rb b/app/forms/llm_connections/feature_binding_form.rb index 4895870c7e8c..b2553f40d105 100644 --- a/app/forms/llm_connections/feature_binding_form.rb +++ b/app/forms/llm_connections/feature_binding_form.rb @@ -73,8 +73,8 @@ def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: end # Only for an embedding feature, and only while unlocked. A locked binding - # renders these as text instead: a disabled input submits nothing, so the - # values would arrive blank and wipe the columns. + # renders it as text instead: a disabled input submits nothing, so the + # value would arrive blank and wipe the column. if embedding && !locked f.text_field( name: :dimensions, @@ -85,22 +85,6 @@ def initialize(options:, inherit_label:, feature_key:, locked: false, embedding: input_width: :small, data: { test_selector: "llm-feature-binding--dimensions-#{feature_key}" } ) - - f.text_field( - name: :input_prefix, - label: LlmFeatureBinding.human_attribute_name(:input_prefix), - caption: I18n.t("admin.llm_feature_bindings.form.input_prefix_caption"), - input_width: :medium, - data: { test_selector: "llm-feature-binding--input-prefix-#{feature_key}" } - ) - - f.text_field( - name: :query_prefix, - label: LlmFeatureBinding.human_attribute_name(:query_prefix), - caption: I18n.t("admin.llm_feature_bindings.form.query_prefix_caption"), - input_width: :medium, - data: { test_selector: "llm-feature-binding--query-prefix-#{feature_key}" } - ) end unless locked diff --git a/app/models/llm_feature_binding.rb b/app/models/llm_feature_binding.rb index 321b30ad6106..dd3c540c553d 100644 --- a/app/models/llm_feature_binding.rb +++ b/app/models/llm_feature_binding.rb @@ -38,7 +38,7 @@ class LlmFeatureBinding < ApplicationRecord # Settings that describe how vectors are written, and so only mean anything # for an embedding feature. - EMBEDDING_SETTINGS = %i[dimensions input_prefix query_prefix].freeze + EMBEDDING_SETTINGS = %i[dimensions].freeze # Everything a stored index depends on. Changing any of it invalidates the # vectors already written, not just the model. @@ -113,11 +113,6 @@ def embedding_settings_only_for_embedding_features # the dimension count is baked into the index, so a locked binding can only be # changed by an explicit re-index. # - # The prefixes are locked for the same reason and matter just as much: an index - # built with "passage: " but queried under a different prefix does not error, - # it quietly returns worse results, which is the hardest kind of failure to - # notice. - # # TODO(#69620): re-indexing is what clears locked_at. Until that job exists a # locked binding can only be changed in the database. def locked_settings_unchanged diff --git a/config/locales/en.yml b/config/locales/en.yml index a347f1ec973a..cac2a8cf7b6f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -217,10 +217,8 @@ en: llm_feature_binding: dimensions: "Dimensions" feature_key: "Feature" - input_prefix: "Document prefix" # ActiveRecord::Base.human_attribute_name strips the _id suffix. model: "Model" - query_prefix: "Query prefix" llm_model: display_name: "Display name" # human_attribute_name strips the _id suffix, so the key omits it. @@ -725,14 +723,8 @@ en: not_for_chat_feature: "only applies to features that create embeddings." feature_key: not_registered: "does not belong to a known AI feature." - input_prefix: - locked: "cannot be changed while data indexed with it still exists. Re-index to change it." - not_for_chat_feature: "only applies to features that create embeddings." model_id: locked: "cannot be changed while data indexed with it still exists. Re-index to switch models." - query_prefix: - locked: "cannot be changed while data indexed with it still exists. Re-index to change it." - not_for_chat_feature: "only applies to features that create embeddings." meeting: error_conflict: "Unable to save because the meeting was updated by someone else in the meantime. Please reload the page." member: @@ -1785,9 +1777,7 @@ en: form: dimensions_caption: "How many numbers each vector has. Leave blank to use whatever the server returns." dimensions_caption_probed: "The server returned %{dimensions}-dimension vectors for this model. Leave blank to use whatever it returns at index time." - input_prefix_caption: "Prepended to each document before it is indexed. Preset for this feature; some models expect their own, for example \"passage: \" including the trailing space." model_caption_embedding: "Only models known to create embeddings are offered. [Read more](docs_url) about model types." - query_prefix_caption: "Prepended to each search query. Preset for this feature; some models expect their own, for example \"query: \" including the trailing space." index: description: "Choose which model each AI feature uses. Features without a choice use the default models set on the [LLMs](models_url) tab." inherit_with_default: "Use the default (%{model})" diff --git a/db/migrate/20260908090000_remove_prefixes_from_llm_feature_bindings.rb b/db/migrate/20260908090000_remove_prefixes_from_llm_feature_bindings.rb new file mode 100644 index 000000000000..0bd8d128b942 --- /dev/null +++ b/db/migrate/20260908090000_remove_prefixes_from_llm_feature_bindings.rb @@ -0,0 +1,38 @@ +# 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. +#++ + +class RemovePrefixesFromLlmFeatureBindings < ActiveRecord::Migration[8.1] + def change + change_table :llm_feature_bindings, bulk: true do |t| + t.remove :input_prefix, type: :string + t.remove :query_prefix, type: :string + end + end +end diff --git a/lib_static/open_project/llm/features.rb b/lib_static/open_project/llm/features.rb index d694a3520b0b..5165d24ca2e8 100644 --- a/lib_static/open_project/llm/features.rb +++ b/lib_static/open_project/llm/features.rb @@ -37,8 +37,7 @@ class UnknownFeature < StandardError; end # Features declare the capabilities they need so the administration UI can # tell an administrator which models are usable for which job, and so a # feature never silently runs against a model that cannot serve it. - Feature = Data.define(:key, :kind, :requires, :prefers, :overridable, :pinned, :available, :i18n_scope, - :input_prefix, :query_prefix) do + Feature = Data.define(:key, :kind, :requires, :prefers, :overridable, :pinned, :available, :i18n_scope) do def available? = available.call def chat? = kind == :chat @@ -83,19 +82,14 @@ def register(key, overridable: false, pinned: false, available: -> { true }, - i18n_scope: nil, - input_prefix: nil, - query_prefix: nil) + i18n_scope: nil) key = key.to_sym validate!(key, kind, requires + prefers) - validate_prefixes!(key, kind, [input_prefix, query_prefix]) all[key] = Feature.new(key:, kind:, requires: requires.map(&:to_sym).freeze, prefers: prefers.map(&:to_sym).freeze, overridable:, pinned:, available:, - i18n_scope: i18n_scope || "llm.features.#{key}", - input_prefix: prefix_for(kind, key, input_prefix), - query_prefix: prefix_for(kind, key, query_prefix)) + i18n_scope: i18n_scope || "llm.features.#{key}") end def all = @all ||= {} @@ -112,22 +106,6 @@ def available = all.values.select(&:available?) def for_kind(kind) = available.select { |feature| feature.kind == kind } - # Each embedding feature indexes its own documents, so its prefixes are - # derived from the key unless the feature names better ones. Administrators - # then never have to type them, while a model that expects its own, such as - # "passage: ", can still be accommodated on the binding. - def prefix_for(kind, key, given) - return unless kind == :embedding - - given || "#{key}_" - end - - def validate_prefixes!(key, kind, prefixes) - return if kind == :embedding || prefixes.none? - - raise ArgumentError, "prefixes are not valid for the chat feature #{key}" - end - def validate!(key, kind, capabilities) raise ArgumentError, "unknown kind #{kind.inspect}" unless KINDS.include?(kind) raise ArgumentError, "LLM feature #{key} is already registered" if all.key?(key) diff --git a/spec/lib/open_project/llm/features_spec.rb b/spec/lib/open_project/llm/features_spec.rb index da8f54f1fc55..355c4e80c386 100644 --- a/spec/lib/open_project/llm/features_spec.rb +++ b/spec/lib/open_project/llm/features_spec.rb @@ -35,35 +35,26 @@ after { described_class.all.delete(key) } - describe "prefixes" do - it "derives them from the key of an embedding feature" do - described_class.register(key, kind: :embedding, requires: %i[embeddings]) - - expect(described_class[key].input_prefix).to eq("spec_only_feature_") - expect(described_class[key].query_prefix).to eq("spec_only_feature_") + describe ".register" do + it "refuses a capability the kind cannot have" do + expect { described_class.register(key, kind: :chat, requires: %i[embeddings]) } + .to raise_error(ArgumentError, /embeddings/) end - it "keeps the ones a feature names itself" do - described_class.register(key, kind: :embedding, input_prefix: "passage: ", query_prefix: "query: ") - - expect(described_class[key].input_prefix).to eq("passage: ") - expect(described_class[key].query_prefix).to eq("query: ") + it "refuses an unknown kind" do + expect { described_class.register(key, kind: :completion) } + .to raise_error(ArgumentError, /unknown kind/) end - it "leaves a chat feature without any" do + it "scopes the translations by the key unless told otherwise" do described_class.register(key, kind: :chat) - expect(described_class[key].input_prefix).to be_nil - expect(described_class[key].query_prefix).to be_nil - end - - it "refuses one on a chat feature" do - expect { described_class.register(key, kind: :chat, input_prefix: "passage: ") } - .to raise_error(ArgumentError, /chat feature/) + expect(described_class[key].i18n_scope).to eq("llm.features.spec_only_feature") end + end - it "presets the ones semantic search is registered with" do - expect(described_class[:semantic_search].input_prefix).to eq("semantic_search_") - end + it "registers semantic search as a pinned embedding feature" do + expect(described_class[:semantic_search]) + .to have_attributes(kind: :embedding, pinned: true, requires: %i[embeddings]) end end diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index e7c71bac9cce..724835dd3415 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -180,34 +180,21 @@ def offered_models(feature_key) mock_llm_embeddings_response(base_url) end - it "stores the vector settings, keeping the prefixes exactly as typed" do + it "stores the vector size" do patch llm_feature_binding_path(:semantic_search), - params: { llm_feature_binding: { model_id: "bge-m3", - dimensions: "1024", - input_prefix: "passage: ", - query_prefix: "query: " } } + params: { llm_feature_binding: { model_id: "bge-m3", dimensions: "1024" } } binding = connection.feature_bindings.find_by(feature_key: "semantic_search") expect(binding.dimensions).to eq(1024) - # The trailing space is load-bearing for the E5 and BGE families. - expect(binding.input_prefix).to eq("passage: ") - expect(binding.query_prefix).to eq("query: ") end - # Typing a prefix by hand is a chore nobody should have to get right, so the - # registration supplies one and an untouched save stores it. - it "prefills the prefixes from the feature registration" do + # The embedder owns them, and nothing read them here. + it "offers no document or query prefix" do get llm_feature_bindings_path - expect(response.body).to include('value="semantic_search_"') - end - - it "stores a cleared prefix as empty rather than restoring the default" do - patch llm_feature_binding_path(:semantic_search), - params: { llm_feature_binding: { model_id: "bge-m3", input_prefix: "" } } - - expect(connection.feature_bindings.find_by(feature_key: "semantic_search").input_prefix).to eq("") + expect(response.body).not_to include("input_prefix") + expect(response.body).not_to include("query_prefix") end it "rejects a dimension count that is not a positive integer" do @@ -231,15 +218,14 @@ def offered_models(feature_key) # index depends on is frozen, not just the model. it "refuses to change anything a locked index depends on" do binding = connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "bge-m3", - dimensions: 1024, input_prefix: "passage: ", - locked_at: Time.current) + dimensions: 1024, locked_at: Time.current) patch llm_feature_binding_path(:semantic_search), - params: { llm_feature_binding: { model_id: "bge-m3", dimensions: "512", input_prefix: "other: " } } + params: { llm_feature_binding: { model_id: "qwen3.6-27b", dimensions: "512" } } binding.reload + expect(binding.model_id).to eq("bge-m3") expect(binding.dimensions).to eq(1024) - expect(binding.input_prefix).to eq("passage: ") end end end From 5991d2b232d9b85c4063079e6c6433dfd04d7e7c Mon Sep 17 00:00:00 2001 From: Andreas Eiselt Date: Tue, 8 Sep 2026 18:20:00 +0200 Subject: [PATCH 10/10] [AI-3] Resolve the feature defaults through the model rows The feature configuration and the runtime still looked their default models up by name, which the connection now stores as references. The embedding picker offers rows, the runtime reads the identifier off the referenced row before asking the server for it, and both default validations work on the row rather than searching the catalogue for a string. The feature configuration and the runtime also ask the setting whether the AI features are on, rather than the column that used to answer it, and the connection factory gained transients so a spec can name the default it wants without knowing the row id. --- .../llm_connections/base_contract.rb | 10 +++--- .../admin/llm_feature_bindings_controller.rb | 4 +-- app/services/llm/runtime.rb | 6 ++-- spec/factories/llm_connection_factory.rb | 14 +++++++- spec/features/admin/llm_connection_spec.rb | 2 +- spec/models/llm_model_spec.rb | 3 +- .../admin/llm_feature_bindings_spec.rb | 12 ++++--- spec/requests/admin/llm_models_spec.rb | 32 ++++++++++--------- .../env_data/llm_connection_seeder_spec.rb | 2 +- spec/services/llm/runtime_spec.rb | 25 ++++++++------- 10 files changed, 64 insertions(+), 46 deletions(-) diff --git a/app/contracts/llm_connections/base_contract.rb b/app/contracts/llm_connections/base_contract.rb index 0bb8088e1447..53bd58a67c46 100644 --- a/app/contracts/llm_connections/base_contract.rb +++ b/app/contracts/llm_connections/base_contract.rb @@ -70,13 +70,11 @@ def not_configured_from_env # would otherwise fail on a catalogue row nothing has probed yet. The picker # still offers confirmed models only. def default_embedding_model_can_embed - model_id = model.default_embedding_model_id - return if model_id.blank? + llm_model = model.default_embedding_model + return if llm_model.blank? return unless model.changed_attributes.include?("default_embedding_model_id") - llm_model = model.models.find_by(external_id: model_id) - - errors.add(:default_embedding_model_id, :cannot_embed) if llm_model&.verdict_for(:embeddings)&.blocking? + errors.add(:default_embedding_model_id, :cannot_embed) if llm_model.verdict_for(:embeddings)&.blocking? end # A model the server identifies as an embedding model is not a chat candidate. @@ -85,7 +83,7 @@ def default_chat_model_can_chat return if llm_model.blank? return unless model.changed_attributes.include?("default_chat_model_id") - errors.add(:default_chat_model_id, :cannot_chat) if model.default_chat_model&.embedding? + errors.add(:default_chat_model_id, :cannot_chat) if llm_model.embedding? end def features_require_connection diff --git a/app/controllers/admin/llm_feature_bindings_controller.rb b/app/controllers/admin/llm_feature_bindings_controller.rb index 9be84933659d..845d45432de3 100644 --- a/app/controllers/admin/llm_feature_bindings_controller.rb +++ b/app/controllers/admin/llm_feature_bindings_controller.rb @@ -56,11 +56,11 @@ def update private def set_connection - @connection = LlmConnection.instance + @connection = LlmConnection.active_connection end def require_enabled_connection - return if @connection.enabled? + return if Setting.llm_features_enabled? && @connection.configured? flash[:notice] = t("admin.llm_connections.disabled_notice") redirect_to llm_connection_path, status: :see_other diff --git a/app/services/llm/runtime.rb b/app/services/llm/runtime.rb index 1c2d9027bab2..b63b41275d15 100644 --- a/app/services/llm/runtime.rb +++ b/app/services/llm/runtime.rb @@ -117,7 +117,7 @@ def call attr_reader :feature, :override def connection - @connection ||= LlmConnection.instance + @connection ||= LlmConnection.active_connection end def resolved_model_id @@ -138,7 +138,9 @@ def binding_model_id end def connection_default - feature.embedding? ? connection.default_embedding_model_id : connection.default_chat_model_id + default = feature.embedding? ? connection.default_embedding_model : connection.default_chat_model + + default&.external_id end # Only a definite :unsupported blocks. An :unknown verdict -- which is the diff --git a/spec/factories/llm_connection_factory.rb b/spec/factories/llm_connection_factory.rb index c18f1b837c1e..a293e26eccc6 100644 --- a/spec/factories/llm_connection_factory.rb +++ b/spec/factories/llm_connection_factory.rb @@ -34,14 +34,26 @@ base_url { "https://example.com/v1" } api_key { "sk-test-key" } trait :with_models do + transient do + default_chat_model_identifier { nil } + default_embedding_model_identifier { nil } + end + catalogue_fetched_at { Time.current } last_connected_at { Time.current } - after(:create) do |connection| + after(:create) do |connection, evaluator| create(:llm_model, llm_connection: connection, external_id: "qwen3.6-27b", raw_metadata: { "owned_by" => "vllm", "max_model_len" => 262_144 }) create(:llm_model, llm_connection: connection, external_id: "bge-m3", raw_metadata: { "owned_by" => "vllm", "max_model_len" => 8_192 }) + + defaults = { default_chat_model: evaluator.default_chat_model_identifier, + default_embedding_model: evaluator.default_embedding_model_identifier } + .compact + .transform_values { |id| connection.models.find_by!(external_id: id) } + + connection.update!(defaults) if defaults.any? end end end diff --git a/spec/features/admin/llm_connection_spec.rb b/spec/features/admin/llm_connection_spec.rb index f48283610d91..4ee9715b104b 100644 --- a/spec/features/admin/llm_connection_spec.rb +++ b/spec/features/admin/llm_connection_spec.rb @@ -239,7 +239,7 @@ def choose_action(item) end describe "the Feature configuration tab" do - let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, base_url:) } before { mock_llm_embeddings_response(base_url) } diff --git a/spec/models/llm_model_spec.rb b/spec/models/llm_model_spec.rb index 1416ffc710f6..26086caeb4d0 100644 --- a/spec/models/llm_model_spec.rb +++ b/spec/models/llm_model_spec.rb @@ -70,7 +70,8 @@ # The decision that makes the toggle safe: curation, not enforcement. A row an # administrator switches off must never silently break a running feature. - it "stays addressable for a feature that is already bound to it" do + it "stays addressable for a feature that is already bound to it", + with_settings: { llm_features_enabled: true } do connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "qwen3.6-27b") expect(connection.available_model_ids).to include("qwen3.6-27b") diff --git a/spec/requests/admin/llm_feature_bindings_spec.rb b/spec/requests/admin/llm_feature_bindings_spec.rb index 724835dd3415..5d22fbb56800 100644 --- a/spec/requests/admin/llm_feature_bindings_spec.rb +++ b/spec/requests/admin/llm_feature_bindings_spec.rb @@ -31,7 +31,8 @@ require "spec_helper" RSpec.describe "Admin AI feature configuration", :llm_server_helpers, :skip_csrf, :webmock, - type: :rails_request, with_flag: { llm_connection: true } do + type: :rails_request, with_flag: { llm_connection: true }, + with_settings: { llm_features_enabled: true } do let(:admin) { create(:admin) } let(:base_url) { "https://example.com/v1" } @@ -52,7 +53,8 @@ def offered_models(feature_key) expect(response).to redirect_to(llm_connection_path) end - it "sends the administrator to the settings while the connection is disabled" do + 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:) get llm_feature_bindings_path @@ -62,7 +64,7 @@ def offered_models(feature_key) end context "with a configured connection" do - let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, base_url:) } it "lists every registered feature" do get llm_feature_bindings_path @@ -120,7 +122,7 @@ def offered_models(feature_key) end describe "PATCH /admin/llm_feature_bindings/:id" do - let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, base_url:) } before { login_as admin } @@ -172,7 +174,7 @@ def offered_models(feature_key) end describe "embedding settings" do - let!(:connection) { create(:llm_connection, :with_models, :enabled, base_url:) } + let!(:connection) { create(:llm_connection, :with_models, base_url:) } before do login_as admin diff --git a/spec/requests/admin/llm_models_spec.rb b/spec/requests/admin/llm_models_spec.rb index de626103f2a4..ed7cac8ae54d 100644 --- a/spec/requests/admin/llm_models_spec.rb +++ b/spec/requests/admin/llm_models_spec.rb @@ -173,7 +173,7 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) end it "offers only models known to embed as the default embedding model" do - connection = create(:llm_connection, :with_models, :enabled, base_url:) + 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) @@ -186,7 +186,7 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) # An unconfirmed capability is not a capability: offering such a model # invites a choice that fails much later, at index time. it "says how to make a model eligible while none is known to embed" do - create(:llm_connection, :with_models, :enabled, base_url:) + create(:llm_connection, :with_models, base_url:) get llm_models_path @@ -197,10 +197,10 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) # an administrator simply hid it, and saying otherwise sends them to the # model form to fix a type that is already right. it "tells a switched-off embedding default apart from an unqualified one" do - connection = create(:llm_connection, :with_models, :enabled, base_url:) + 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) - connection.update_column(:default_embedding_model_id, "bge-m3") + connection.update_column(:default_embedding_model_id, connection.models.find_by(external_id: "bge-m3").id) connection.models.find_by(external_id: "bge-m3").update!(deactivated_at: Time.current) get llm_models_path @@ -212,7 +212,7 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) # The remedy the caption names is only the right one while nothing embeds. it "keeps the documentation caption while a known embedding model is switched off" do - connection = create(:llm_connection, :with_models, :enabled, base_url:) + 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) connection.models.find_by(external_id: "bge-m3").update!(deactivated_at: Time.current) @@ -225,8 +225,8 @@ def streamed_markup = Capybara.string(response.body.gsub(%r{}, "")) # Otherwise a save would silently blank a working configuration. it "keeps the stored embedding default listed, flagged, once it is ruled out" do - connection = create(:llm_connection, :with_models, :enabled, base_url:) - connection.update_column(:default_embedding_model_id, "qwen3.6-27b") + connection = create(:llm_connection, :with_models, base_url:) + connection.update_column(:default_embedding_model_id, connection.models.find_by(external_id: "qwen3.6-27b").id) get llm_models_path @@ -697,8 +697,6 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size end it "keeps the feature resolving afterwards", with_flag: { llm_connection: true } do - connection.update!(enabled: true) - patch llm_model_path(llm_model), params: { llm_model: { external_id: "qwen/qwen3.6-35b-a3b:bf16" } } expect(Llm::Runtime.for(:description_assistant).model_id).to eq("qwen/qwen3.6-35b-a3b:bf16") @@ -786,16 +784,18 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size connection.capability_verdicts.create!(model_id: "bge-m3", capability: "embeddings", state: "supported", source: "probe", checked_at: Time.current) - patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "bge-m3" } } + patch defaults_llm_models_path, + params: { llm_connection: { default_embedding_model_id: connection.models.find_by(external_id: "bge-m3").id } } - expect(connection.reload.default_embedding_model_id).to eq("bge-m3") + expect(connection.reload.default_embedding_model.external_id).to eq("bge-m3") end it "refuses a model the server has ruled out" do connection.capability_verdicts.create!(model_id: "qwen3.6-27b", capability: "embeddings", state: "unsupported", source: "probe", checked_at: Time.current) - patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "qwen3.6-27b" } } + patch defaults_llm_models_path, + params: { llm_connection: { default_embedding_model_id: connection.models.find_by(external_id: "qwen3.6-27b").id } } expect(connection.reload.default_embedding_model_id).to be_nil expect(flash[:error]).to be_present @@ -804,9 +804,10 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size # Nothing has probed the row: refusing here would break provisioning from the # environment, where the same configuration passes on an empty catalogue. it "accepts a model nothing has ruled out" do - patch defaults_llm_models_path, params: { llm_connection: { default_embedding_model_id: "qwen3.6-27b" } } + patch defaults_llm_models_path, + params: { llm_connection: { default_embedding_model_id: connection.models.find_by(external_id: "qwen3.6-27b").id } } - expect(connection.reload.default_embedding_model_id).to eq("qwen3.6-27b") + expect(connection.reload.default_embedding_model.external_id).to eq("qwen3.6-27b") end it "leaves the server settings alone" do @@ -819,7 +820,8 @@ def rendered_rows(body) = body.scan("llm-model--toggle-").size it "refuses a default the environment owns" do allow(Setting).to receive(:llm_connection).and_return({ "base_url" => base_url }) - patch defaults_llm_models_path, params: { llm_connection: { default_chat_model_id: "qwen3.6-27b" } } + patch defaults_llm_models_path, + params: { llm_connection: { default_chat_model_id: connection.models.find_by(external_id: "qwen3.6-27b").id } } expect(connection.reload.default_chat_model_id).to be_nil expect(flash[:error]).to be_present diff --git a/spec/seeders/env_data/llm_connection_seeder_spec.rb b/spec/seeders/env_data/llm_connection_seeder_spec.rb index 9e76c6c50b0d..747eb40d4157 100644 --- a/spec/seeders/env_data/llm_connection_seeder_spec.rb +++ b/spec/seeders/env_data/llm_connection_seeder_spec.rb @@ -96,7 +96,7 @@ it "provisions the default" do seed - expect(LlmConnection.first.default_embedding_model_id).to eq("bge-m3") + expect(LlmConnection.first.default_embedding_model.external_id).to eq("bge-m3") end end diff --git a/spec/services/llm/runtime_spec.rb b/spec/services/llm/runtime_spec.rb index d462972431e7..72805b88c379 100644 --- a/spec/services/llm/runtime_spec.rb +++ b/spec/services/llm/runtime_spec.rb @@ -30,7 +30,8 @@ require "spec_helper" -RSpec.describe Llm::Runtime, with_flag: { llm_connection: true } do +RSpec.describe Llm::Runtime, with_flag: { llm_connection: true }, + with_settings: { llm_features_enabled: true } do subject(:resolution) { described_class.for(feature_key, override:) } let(:feature_key) { :description_assistant } @@ -40,14 +41,14 @@ it { expect(resolution.status).to eq(:no_connection) } end - context "with a connection that is not enabled" do - before { create(:llm_connection, :with_models, enabled: false) } + context "with the AI features switched off", with_settings: { llm_features_enabled: false } do + before { create(:llm_connection, :with_models) } it { expect(resolution.status).to eq(:no_connection) } end - context "with an enabled connection" do - let!(:connection) { create(:llm_connection, :with_models, :enabled) } + context "with the AI features switched on" do + let!(:connection) { create(:llm_connection, :with_models) } it "is unbound until a model is chosen" do expect(resolution.status).to eq(:unbound) @@ -55,14 +56,14 @@ end it "falls back to the connection default" do - connection.update!(default_chat_model_id: "qwen3.6-27b") + connection.update!(default_chat_model: connection.models.find_by(external_id: "qwen3.6-27b")) expect(resolution).to be_ready expect(resolution.model_id).to eq("qwen3.6-27b") end it "prefers the feature binding over the connection default" do - connection.update!(default_chat_model_id: "qwen3.6-27b") + connection.update!(default_chat_model: connection.models.find_by(external_id: "qwen3.6-27b")) connection.feature_bindings.create!(feature_key: "description_assistant", model_id: "bge-m3") expect(resolution.model_id).to eq("bge-m3") @@ -95,7 +96,7 @@ context "when the chosen model is gone from the catalogue" do let(:override) { "vanished-model" } - before { connection.update!(default_chat_model_id: "qwen3.6-27b") } + before { connection.update!(default_chat_model: connection.models.find_by(external_id: "qwen3.6-27b")) } it "fails closed rather than falling back" do expect(resolution.status).to eq(:model_missing) @@ -106,7 +107,7 @@ describe "capability gating" do let(:feature_key) { :semantic_search } - let!(:connection) { create(:llm_connection, :with_models, :enabled) } + let!(:connection) { create(:llm_connection, :with_models) } before { connection.feature_bindings.create!(feature_key: "semantic_search", model_id: "qwen3.6-27b") } @@ -133,7 +134,7 @@ end describe "running a request", :llm_server_helpers, :webmock do - let!(:connection) { create(:llm_connection, :with_models, :enabled, default_chat_model_id: "qwen3.6-27b") } + let!(:connection) { create(:llm_connection, :with_models, default_chat_model_identifier: "qwen3.6-27b") } it "sends a completion for the resolved model" do mock_llm_chat_response("https://example.com/v1", content: "pong") @@ -144,7 +145,7 @@ end it "refuses when the feature is not ready" do - connection.update!(enabled: false) + allow(Setting).to receive(:llm_features_enabled?).and_return(false) expect { resolution.chat }.to raise_error(Llm::Errors::NotReady) { |e| expect(e.status).to eq(:no_connection) } end @@ -159,7 +160,7 @@ context "with an embedding feature" do let(:feature_key) { :semantic_search } - before { connection.update!(default_embedding_model_id: "bge-m3") } + before { connection.update!(default_embedding_model: connection.models.find_by(external_id: "bge-m3")) } it "requests a vector for the resolved model" do mock_llm_embeddings_response("https://example.com/v1", dimensions: 8)