From e7f3d68684124620f7e5e7b6d9f9df79dd6da698 Mon Sep 17 00:00:00 2001 From: Cindy Halim Date: Fri, 17 Oct 2025 16:13:26 -0400 Subject: [PATCH 1/3] Remove instance_level_model_selection flag references --- .../mutations/ai/feature_settings/update.rb | 2 - .../feature_settings_resolver.rb | 2 - .../admin/ai/self_hosted_models_helper.rb | 1 - ...nstance_model_selection_feature_setting.rb | 7 +- ee/app/policies/ee/global_policy.rb | 2 - .../ai/feature_setting_selection_service.rb | 31 ++-- ...te_self_managed_model_selection_service.rb | 2 +- ee/lib/code_suggestions/model_details/base.rb | 1 - .../model_details/code_completion.rb | 6 +- .../code_completion/self_hosted/vendored.rb | 14 +- .../ai/self_hosted_models_helper_spec.rb | 8 - .../model_details/base_spec.rb | 30 ++-- .../model_details/code_completion_spec.rb | 68 ++++---- ...ce_model_selection_feature_setting_spec.rb | 91 ++-------- ee/spec/policies/global_policy_spec.rb | 13 +- .../feature_settings_resolver_spec.rb | 11 -- .../ai/feature_settings/update_spec.rb | 10 -- .../feature_setting_selection_service_spec.rb | 165 +++++++----------- ...lf_managed_model_selection_service_spec.rb | 126 ++++++------- 19 files changed, 199 insertions(+), 391 deletions(-) diff --git a/ee/app/graphql/mutations/ai/feature_settings/update.rb b/ee/app/graphql/mutations/ai/feature_settings/update.rb index fec14f45cc5829..c59f4d823a2c17 100644 --- a/ee/app/graphql/mutations/ai/feature_settings/update.rb +++ b/ee/app/graphql/mutations/ai/feature_settings/update.rb @@ -70,8 +70,6 @@ def update_model_selection(feature, args) end def gitlab_model_definitions - return unless Feature.enabled?(:instance_level_model_selection, :instance) - response = ::Ai::ModelSelection::FetchModelDefinitionsService .new(current_user, model_selection_scope: nil) .execute diff --git a/ee/app/graphql/resolvers/ai/feature_settings/feature_settings_resolver.rb b/ee/app/graphql/resolvers/ai/feature_settings/feature_settings_resolver.rb index f620ae7bd795bc..a36b3edf3025a9 100644 --- a/ee/app/graphql/resolvers/ai/feature_settings/feature_settings_resolver.rb +++ b/ee/app/graphql/resolvers/ai/feature_settings/feature_settings_resolver.rb @@ -46,8 +46,6 @@ def gitlab_models? end def gitlab_model_definitions - return unless Feature.enabled?(:instance_level_model_selection, :instance) - result = ::Ai::ModelSelection::FetchModelDefinitionsService .new(current_user, model_selection_scope: nil) .execute diff --git a/ee/app/helpers/admin/ai/self_hosted_models_helper.rb b/ee/app/helpers/admin/ai/self_hosted_models_helper.rb index ee8666f79e25b4..8c5342842d0bee 100644 --- a/ee/app/helpers/admin/ai/self_hosted_models_helper.rb +++ b/ee/app/helpers/admin/ai/self_hosted_models_helper.rb @@ -38,7 +38,6 @@ def dedicated_instance? end def show_self_hosted_vendored_model_option? - return false if ::Feature.enabled?(:instance_level_model_selection, :instance) return false unless ::Feature.enabled?(:ai_self_hosted_vendored_features, current_user) !!::License.current&.online_cloud_license? diff --git a/ee/app/models/ai/model_selection/instance_model_selection_feature_setting.rb b/ee/app/models/ai/model_selection/instance_model_selection_feature_setting.rb index 5438ee81c85cf0..edfd4e6aa0b62e 100644 --- a/ee/app/models/ai/model_selection/instance_model_selection_feature_setting.rb +++ b/ee/app/models/ai/model_selection/instance_model_selection_feature_setting.rb @@ -12,11 +12,6 @@ class InstanceModelSelectionFeatureSetting < ApplicationRecord scope :non_default, -> { where.not(offered_model_ref: [nil, ""]) } def self.find_or_initialize_by_feature(feature) - # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Instance-level feature flag for global model selection settings - return unless ::Feature.enabled?(:instance_level_model_selection) - - # rubocop:enable Gitlab/FeatureFlagWithoutActor - feature_name = get_feature_name(feature) find_or_initialize_by(feature: feature_name) end @@ -36,7 +31,7 @@ def vendored? private def model_selection_enabled? - ::Feature.enabled?(:instance_level_model_selection, model_selection_scope) + true end end end diff --git a/ee/app/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb index 006deb67dea29f..c7c48169ac51fc 100644 --- a/ee/app/policies/ee/global_policy.rb +++ b/ee/app/policies/ee/global_policy.rb @@ -122,8 +122,6 @@ module GlobalPolicy end condition(:instance_model_selection_available) do - next false unless ::Feature.enabled?(:instance_level_model_selection, :instance) - !::License.current&.offline_cloud_license? end diff --git a/ee/app/services/ai/feature_setting_selection_service.rb b/ee/app/services/ai/feature_setting_selection_service.rb index 3817f115a0e9c8..3f2c6f8ec681b8 100644 --- a/ee/app/services/ai/feature_setting_selection_service.rb +++ b/ee/app/services/ai/feature_setting_selection_service.rb @@ -27,24 +27,21 @@ def execute # First check if a self-hosted model is defined for the feature feature_setting = self_hosted_feature_setting - if ::Feature.enabled?(:instance_level_model_selection, :instance) - return ServiceResponse.success(payload: feature_setting) if feature_setting && !feature_setting.vendored? - - instance_setting = instance_level_setting - - # When a Self-hosted AI Gateway has been configured (for the instance), then we don't default to vendored - # vendored becomes the default only on pure cloud-connected SM/dedicated instances. The same for offline - # cloud license, since there's no connection to vendored models - if ::License.current&.offline_cloud_license? || - (instance_setting.set_to_gitlab_default? && Gitlab::AiGateway.has_self_hosted_ai_gateway?) - return ServiceResponse.success(payload: nil) - end - - # Instance level is fetched either when we don't have a feature_setting, or when it is set to vendored - ServiceResponse.success(payload: instance_setting) - else - ServiceResponse.success(payload: feature_setting) + return ServiceResponse.success(payload: feature_setting) if feature_setting && !feature_setting.vendored? + + instance_setting = instance_level_setting + + # When a Self-hosted AI Gateway has been configured (for the instance), then we don't default to vendored + # vendored becomes the default only on pure cloud-connected SM/dedicated instances. The same for offline + # cloud license, since there's no connection to vendored models + if ::License.current&.offline_cloud_license? || + (instance_setting.set_to_gitlab_default? && Gitlab::AiGateway.has_self_hosted_ai_gateway?) + return ServiceResponse.success(payload: nil) end + + # Instance level is fetched either when we don't have a feature_setting, or when it is set to vendored + ServiceResponse.success(payload: instance_setting) + end end diff --git a/ee/app/services/ai/model_selection/update_self_managed_model_selection_service.rb b/ee/app/services/ai/model_selection/update_self_managed_model_selection_service.rb index b7025b24a83e33..84aa3dd7e9d818 100644 --- a/ee/app/services/ai/model_selection/update_self_managed_model_selection_service.rb +++ b/ee/app/services/ai/model_selection/update_self_managed_model_selection_service.rb @@ -9,7 +9,7 @@ def initialize(user, params) end def execute - if ::Feature.enabled?(:instance_level_model_selection, :instance) && params[:provider] == "vendored" + if params[:provider] == "vendored" # Since feature_setting (Duo self-hosted) takes precedence when resolving which feature setting to use, we # must set ai_self_hosted_model_id to nil so that it is properly defined as vendored and should be handled # at the instance level model selection diff --git a/ee/lib/code_suggestions/model_details/base.rb b/ee/lib/code_suggestions/model_details/base.rb index a49f1e6dc39df1..d6130a970c5f2f 100644 --- a/ee/lib/code_suggestions/model_details/base.rb +++ b/ee/lib/code_suggestions/model_details/base.rb @@ -59,7 +59,6 @@ def default? return true unless feature_setting.present? return false unless vendored? return true if feature_setting.is_a?(Ai::FeatureSetting) - return true if ::Feature.disabled?(:instance_level_model_selection, :instance) feature_setting.set_to_gitlab_default? end diff --git a/ee/lib/code_suggestions/model_details/code_completion.rb b/ee/lib/code_suggestions/model_details/code_completion.rb index 625290fbbbcf4f..621eeba3d64674 100644 --- a/ee/lib/code_suggestions/model_details/code_completion.rb +++ b/ee/lib/code_suggestions/model_details/code_completion.rb @@ -27,11 +27,7 @@ def current_model # if self-hosted, the model details are provided by the client return {} if self_hosted? - if Feature.enabled?(:instance_level_model_selection, :instance) - return params_from_feature_setting(feature_setting) unless Gitlab.org_or_com? || default? # rubocop: disable Gitlab/AvoidGitlabInstanceChecks -- This is already used in this class - elsif vendored? - return params_as_if_gitlab_default_model(FEATURE_SETTING_NAME) - end + return params_from_feature_setting(feature_setting) unless Gitlab.org_or_com? || default? # rubocop: disable Gitlab/AvoidGitlabInstanceChecks -- This is already used in this class return vertex_codestral_2501_model_details if code_completion_opt_out_fireworks? diff --git a/ee/lib/code_suggestions/prompts/code_completion/self_hosted/vendored.rb b/ee/lib/code_suggestions/prompts/code_completion/self_hosted/vendored.rb index 58458ffe1d2d9a..58a1ad1f1879b3 100644 --- a/ee/lib/code_suggestions/prompts/code_completion/self_hosted/vendored.rb +++ b/ee/lib/code_suggestions/prompts/code_completion/self_hosted/vendored.rb @@ -8,16 +8,10 @@ class Vendored < CodeSuggestions::Prompts::Base include ::Ai::ModelSelection::Concerns::GitlabDefaultModelParams def request_params - if Feature.enabled?(:instance_level_model_selection, :instance) - return { - model_provider: "gitlab", - model_name: feature_setting&.offered_model_ref || '' - } - end - - params_as_if_gitlab_default_model( - ::CodeSuggestions::ModelDetails::CodeCompletion::FEATURE_SETTING_NAME - ) + { + model_provider: "gitlab", + model_name: feature_setting&.offered_model_ref || '' + } end end end diff --git a/ee/spec/helpers/admin/ai/self_hosted_models_helper_spec.rb b/ee/spec/helpers/admin/ai/self_hosted_models_helper_spec.rb index fd65520c333299..39c823bc9b0666 100644 --- a/ee/spec/helpers/admin/ai/self_hosted_models_helper_spec.rb +++ b/ee/spec/helpers/admin/ai/self_hosted_models_helper_spec.rb @@ -54,14 +54,7 @@ end describe "#show_self_hosted_vendored_model_option?" do - it "returns false when the instance-level model selection flag is enabled" do - stub_feature_flags(instance_level_model_selection: true) - - expect(helper.show_self_hosted_vendored_model_option?).to be(false) - end - it "returns false when the vendored model option feature flag is disabled" do - stub_feature_flags(instance_level_model_selection: false) stub_feature_flags(ai_self_hosted_vendored_features: false) expect(helper.show_self_hosted_vendored_model_option?).to be(false) @@ -71,7 +64,6 @@ subject { helper.show_self_hosted_vendored_model_option? } before do - stub_feature_flags(instance_level_model_selection: false) stub_feature_flags(ai_self_hosted_vendored_features: true) end diff --git a/ee/spec/lib/code_suggestions/model_details/base_spec.rb b/ee/spec/lib/code_suggestions/model_details/base_spec.rb index 4207b3bfbd249b..0e4aea96fe3c32 100644 --- a/ee/spec/lib/code_suggestions/model_details/base_spec.rb +++ b/ee/spec/lib/code_suggestions/model_details/base_spec.rb @@ -202,10 +202,6 @@ describe '#default?' do subject(:default?) { model_details.default? } - before do - stub_feature_flags(instance_level_model_selection: false) - end - context 'when the feature setting is nil' do it { is_expected.to be(true) } end @@ -240,28 +236,22 @@ it { is_expected.to be(true) } - context 'when :instance_level_model_selection is enabled' do + context 'when the instance level namespace is default' do before do - stub_feature_flags(instance_level_model_selection: true) + create(:instance_model_selection_feature_setting, feature: feature_setting_name, offered_model_ref: nil) end - context 'when the instance level namespace is default' do - before do - create(:instance_model_selection_feature_setting, feature: feature_setting_name, offered_model_ref: nil) - end + it { is_expected.to be(true) } + end - it { is_expected.to be(true) } + context 'when the instance level namespace is not default' do + before do + create(:instance_model_selection_feature_setting, + feature: feature_setting_name, + offered_model_ref: 'claude_sonnet_3_5') end - context 'when the instance level namespace is not default' do - before do - create(:instance_model_selection_feature_setting, - feature: feature_setting_name, - offered_model_ref: 'claude_sonnet_3_5') - end - - it { is_expected.to be(false) } - end + it { is_expected.to be(false) } end end end diff --git a/ee/spec/lib/code_suggestions/model_details/code_completion_spec.rb b/ee/spec/lib/code_suggestions/model_details/code_completion_spec.rb index a0749e1965cb75..63af6ae257016f 100644 --- a/ee/spec/lib/code_suggestions/model_details/code_completion_spec.rb +++ b/ee/spec/lib/code_suggestions/model_details/code_completion_spec.rb @@ -160,56 +160,48 @@ let(:expected_self_hosted_model_result) { {} } before do - stub_feature_flags( - use_claude_code_completion: false, - instance_level_model_selection: false - ) + stub_feature_flags(use_claude_code_completion: false) end - context 'when instance level model selection is enabled' do + context 'when instance duo self-hosted config exists' do before do - stub_feature_flags( - instance_level_model_selection: true, - code_completion_opt_out_fireworks: false - ) + stub_feature_flags(code_completion_opt_out_fireworks: false) end - context 'when instance duo self-hosted config exists' do - context 'and is not set to vendored' do - let_it_be(:self_hosted_model) { create(:ai_self_hosted_model) } + context 'and is not set to vendored' do + let_it_be(:self_hosted_model) { create(:ai_self_hosted_model) } + before do + create(:ai_feature_setting, + self_hosted_model: self_hosted_model, + provider: :self_hosted, + feature: 'code_completions') + end + + it 'returns empty response' do + expect(actual_result).to eq(expected_self_hosted_model_result) + end + end + + context 'and is set to vendored' do + context 'and instance level is not default' do before do - create(:ai_feature_setting, - self_hosted_model: self_hosted_model, - provider: :self_hosted, - feature: 'code_completions') + create(:instance_model_selection_feature_setting, + feature: 'code_completions', + offered_model_ref: 'claude_sonnet_3_5') end - it 'returns empty response' do - expect(actual_result).to eq(expected_self_hosted_model_result) + it 'returns the selected model' do + expect(actual_result).to eq({ + model_provider: 'gitlab', + model_name: 'claude_sonnet_3_5' + }) end end - context 'and is set to vendored' do - context 'and instance level is not default' do - before do - create(:instance_model_selection_feature_setting, - feature: 'code_completions', - offered_model_ref: 'claude_sonnet_3_5') - end - - it 'returns the selected model' do - expect(actual_result).to eq({ - model_provider: 'gitlab', - model_name: 'claude_sonnet_3_5' - }) - end - end - - context 'and instance level is default' do - it 'returns the default model' do - expect(actual_result).to eq(expected_fireworks_codestral_result) - end + context 'and instance level is default' do + it 'returns the default model' do + expect(actual_result).to eq(expected_fireworks_codestral_result) end end end diff --git a/ee/spec/models/ai/model_selection/instance_model_selection_feature_setting_spec.rb b/ee/spec/models/ai/model_selection/instance_model_selection_feature_setting_spec.rb index 1a5e304bf11cd8..ccd819f3a95653 100644 --- a/ee/spec/models/ai/model_selection/instance_model_selection_feature_setting_spec.rb +++ b/ee/spec/models/ai/model_selection/instance_model_selection_feature_setting_spec.rb @@ -49,57 +49,36 @@ describe '.find_or_initialize_by_feature' do let(:existing_feature) { instance_feature_setting.feature.to_sym } - context 'when instance_level_model_selection feature flag is enabled' do - before do - stub_feature_flags(instance_level_model_selection: true) + context 'when setting exists' do + it 'returns the existing setting' do + instance_feature_setting.save! + result = described_class.find_or_initialize_by_feature(existing_feature) + expect(result).to eq(instance_feature_setting) + expect(result).to be_persisted end + end - context 'when setting exists' do - it 'returns the existing setting' do - instance_feature_setting.save! - result = described_class.find_or_initialize_by_feature(existing_feature) - expect(result).to eq(instance_feature_setting) - expect(result).to be_persisted - end + context 'when setting does not exist' do + it 'returns a new initialized setting' do + result = described_class.find_or_initialize_by_feature(:duo_chat) + expect(result).to be_a(described_class) + expect(result).not_to be_persisted + expect(result.feature).to eq('duo_chat') end + end - context 'when setting does not exist' do - it 'returns a new initialized setting' do - result = described_class.find_or_initialize_by_feature(:duo_chat) - expect(result).to be_a(described_class) - expect(result).not_to be_persisted + context 'for duo chat tools' do + it 'returns the duo chat feature setting for all duo chat tools' do + described_class::DUO_CHAT_TOOLS.each do |feature_sym| + result = described_class.find_or_initialize_by_feature(feature_sym) expect(result.feature).to eq('duo_chat') end end - - context 'for duo chat tools' do - it 'returns the duo chat feature setting for all duo chat tools' do - described_class::DUO_CHAT_TOOLS.each do |feature_sym| - result = described_class.find_or_initialize_by_feature(feature_sym) - expect(result.feature).to eq('duo_chat') - end - end - end - end - - context 'when instance_level_model_selection feature flag is disabled' do - before do - stub_feature_flags(instance_level_model_selection: false) - end - - it 'returns nil' do - result = described_class.find_or_initialize_by_feature(existing_feature) - expect(result).to be_nil - end end context 'when ai_model_switching feature flag is disabled' do let(:ff_enabled) { false } - before do - stub_feature_flags(instance_level_model_selection: true) - end - it 'returns a valid record because instance model selection no longer depends on ai_model_switching' do result = described_class.find_or_initialize_by_feature(existing_feature) expect(result).to be_a(described_class) @@ -235,39 +214,5 @@ end end end - - describe 'when model_selection_enabled?' do - context 'when instance_level_model_selection feature flag is enabled' do - before do - stub_feature_flags(instance_level_model_selection: true) - end - - it 'returns true' do - expect(instance_feature_setting.send(:model_selection_enabled?)).to be true - end - end - - context 'when instance_level_model_selection feature flag is disabled' do - before do - stub_feature_flags(instance_level_model_selection: false) - end - - it 'returns false' do - expect(instance_feature_setting.send(:model_selection_enabled?)).to be false - end - end - - context 'when ai_model_switching feature flag is disabled' do - let(:ff_enabled) { false } - - before do - stub_feature_flags(instance_level_model_selection: true) - end - - it 'returns true because instance model selection does not depend on ai_model_switching' do - expect(instance_feature_setting.send(:model_selection_enabled?)).to be true - end - end - end end end diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb index b2806a937a08b6..746d715aa6400d 100644 --- a/ee/spec/policies/global_policy_spec.rb +++ b/ee/spec/policies/global_policy_spec.rb @@ -895,13 +895,12 @@ end context 'when admin', :enable_admin_mode do - where(:is_licensed, :is_active_add_on, :instance_level_model_selection_enabled, :is_offline_license, + where(:is_licensed, :is_active_add_on, :is_offline_license, :can_manage_instance_model_selection) do - true | true | true | false | be_allowed(:manage_instance_model_selection) - true | false | true | false | be_disallowed(:manage_instance_model_selection) - true | true | false | false | be_disallowed(:manage_instance_model_selection) - true | true | true | true | be_disallowed(:manage_instance_model_selection) - false | true | true | false | be_disallowed(:manage_instance_model_selection) + true | true | false | be_allowed(:manage_instance_model_selection) + true | false | false | be_disallowed(:manage_instance_model_selection) + true | true | true | be_disallowed(:manage_instance_model_selection) + false | true | false | be_disallowed(:manage_instance_model_selection) end with_them do @@ -915,8 +914,6 @@ allow(::GitlabSubscriptions::AddOnPurchase) .to receive_message_chain(:for_self_managed, :for_duo_enterprise, :active, :exists?).and_return(is_active_add_on) - - stub_feature_flags(instance_level_model_selection: instance_level_model_selection_enabled) end it { is_expected.to can_manage_instance_model_selection } diff --git a/ee/spec/requests/api/graphql/ai/feature_settings/feature_settings_resolver_spec.rb b/ee/spec/requests/api/graphql/ai/feature_settings/feature_settings_resolver_spec.rb index 4b85a8b91cb245..93685ec35ef674 100644 --- a/ee/spec/requests/api/graphql/ai/feature_settings/feature_settings_resolver_spec.rb +++ b/ee/spec/requests/api/graphql/ai/feature_settings/feature_settings_resolver_spec.rb @@ -172,16 +172,6 @@ expect(result).to match_array(expected_response) end - - context 'when :instance_level_model_selection feature flag is off' do - before do - stub_feature_flags(instance_level_model_selection: false) - end - - it 'does not make request for model definitions' do - expect(::Ai::ModelSelection::FetchModelDefinitionsService).not_to receive(:new) - end - end end context 'when an Self-hosted model ID query parameters are given' do @@ -265,7 +255,6 @@ ServiceResponse.success(payload: nil) ) - stub_feature_flags(instance_level_model_selection: true) stub_application_setting(duo_features_enabled: false) end diff --git a/ee/spec/requests/api/graphql/ai/feature_settings/update_spec.rb b/ee/spec/requests/api/graphql/ai/feature_settings/update_spec.rb index 7f092379d256ac..4c64e37cea494f 100644 --- a/ee/spec/requests/api/graphql/ai/feature_settings/update_spec.rb +++ b/ee/spec/requests/api/graphql/ai/feature_settings/update_spec.rb @@ -172,16 +172,6 @@ 'ref' => 'claude-sonnet' } ) end - - context 'when :instance_level_model_selection feature flag is off' do - before do - stub_feature_flags(instance_level_model_selection: false) - end - - it 'does not make request for model definitions' do - expect(::Ai::ModelSelection::FetchModelDefinitionsService).not_to receive(:new) - end - end end context 'when the feature setting does not exist' do diff --git a/ee/spec/services/ai/feature_setting_selection_service_spec.rb b/ee/spec/services/ai/feature_setting_selection_service_spec.rb index 13ef87b305fe96..8908c0169802ac 100644 --- a/ee/spec/services/ai/feature_setting_selection_service_spec.rb +++ b/ee/spec/services/ai/feature_setting_selection_service_spec.rb @@ -113,85 +113,63 @@ end context 'when running on self-hosted instance' do - context 'when instance_level_model_selection feature flag is disabled' do - before do - stub_feature_flags(instance_level_model_selection: false) + context 'when self-hosted feature setting exists and is not vendored' do + let_it_be(:ai_feature_setting) do + create(:ai_feature_setting, self_hosted_model: self_hosted_model, feature: :duo_chat) end - context 'when self-hosted feature setting exists' do - let_it_be(:ai_feature_setting) do - create(:ai_feature_setting, self_hosted_model: self_hosted_model, feature: :duo_chat) - end - - it 'returns success with self-hosted feature setting' do - expect(response).to be_success - expect(response.payload).to eq(ai_feature_setting) - end + it 'returns success with self-hosted feature setting' do + expect(response).to be_success + expect(response.payload).to eq(ai_feature_setting) end + end - context 'when self-hosted feature setting exists and is vendored' do - let_it_be(:vendored_feature_setting) do - create(:ai_feature_setting, feature: :duo_chat, provider: :vendored) - end - - it 'returns success with vendored feature setting' do - expect(response).to be_success - expect(response.payload).to eq(vendored_feature_setting) - end + context 'when self-hosted feature setting exists and is vendored' do + let_it_be(:vendored_feature_setting) do + create(:ai_feature_setting, feature: :duo_chat, provider: :vendored) end - context 'when self-hosted feature setting does not exist' do - it 'returns success with nil payload' do - expect(response).to be_success - expect(response.payload).to be_nil - end + it 'returns success with instance level setting' do + expect(response).to be_success + expect(response.payload).to be_a(::Ai::ModelSelection::InstanceModelSelectionFeatureSetting) + expect(response.payload.feature).to eq(feature.to_s) end end - context 'when instance_level_model_selection feature flag is enabled' do - before do - stub_feature_flags(instance_level_model_selection: true) - end - - context 'when self-hosted feature setting exists and is not vendored' do - let_it_be(:ai_feature_setting) do - create(:ai_feature_setting, self_hosted_model: self_hosted_model, feature: :duo_chat) - end - - it 'returns success with self-hosted feature setting' do + context 'when self-hosted feature setting does not exist' do + context 'and self-hosted AI Gateway has not been configured' do + it 'returns success with default instance level setting' do expect(response).to be_success - expect(response.payload).to eq(ai_feature_setting) + expect(response.payload.offered_model_ref).to be_blank + expect(response.payload).not_to be_persisted + expect(response.payload.feature).to eq(feature.to_s) end end - context 'when self-hosted feature setting exists and is vendored' do - let_it_be(:vendored_feature_setting) do - create(:ai_feature_setting, feature: :duo_chat, provider: :vendored) + context 'and instance is on offline cloud license' do + let_it_be(:license) { create(:license) } + + before do + allow(::License).to receive(:current).and_return(license) + allow(license).to receive(:offline_cloud_license?).and_return(true) end - it 'returns success with instance level setting' do + it 'does not create a default vendored instance setting and returns nil instead' do expect(response).to be_success - expect(response.payload).to be_a(::Ai::ModelSelection::InstanceModelSelectionFeatureSetting) - expect(response.payload.feature).to eq(feature.to_s) + expect(response.payload).to be_nil end end - context 'when self-hosted feature setting does not exist' do - context 'and self-hosted AI Gateway has not been configured' do - it 'returns success with default instance level setting' do - expect(response).to be_success - expect(response.payload.offered_model_ref).to be_blank - expect(response.payload).not_to be_persisted - expect(response.payload.feature).to eq(feature.to_s) - end + context 'and self-hosted AI Gateway has been configured' do + before do + create(:ai_settings, ai_gateway_url: 'http://example.com') end - context 'and instance is on offline cloud license' do - let_it_be(:license) { create(:license) } - - before do - allow(::License).to receive(:current).and_return(license) - allow(license).to receive(:offline_cloud_license?).and_return(true) + context 'and instance level is default' do + let_it_be(:instance_setting) do + create(:instance_model_selection_feature_setting, + feature: :duo_chat, + offered_model_ref: nil) end it 'does not create a default vendored instance setting and returns nil instead' do @@ -200,62 +178,43 @@ end end - context 'and self-hosted AI Gateway has been configured' do - before do - create(:ai_settings, ai_gateway_url: 'http://example.com') - end - - context 'and instance level is default' do - let_it_be(:instance_setting) do - create(:instance_model_selection_feature_setting, - feature: :duo_chat, - offered_model_ref: nil) - end - - it 'does not create a default vendored instance setting and returns nil instead' do - expect(response).to be_success - expect(response.payload).to be_nil - end + context 'and instance level is not default' do + let_it_be(:instance_setting) do + create(:instance_model_selection_feature_setting, + feature: :duo_chat) end - context 'and instance level is not default' do - let_it_be(:instance_setting) do - create(:instance_model_selection_feature_setting, - feature: :duo_chat) - end - - it 'returns the existing instance setting' do - expect(response).to be_success - expect(response.payload.offered_model_ref).to eq('claude-3-7-sonnet-20250219') - end + it 'returns the existing instance setting' do + expect(response).to be_success + expect(response.payload.offered_model_ref).to eq('claude-3-7-sonnet-20250219') end end end + end - context 'when instance level setting already exists' do - let_it_be(:instance_setting) do - create(:instance_model_selection_feature_setting, feature: :duo_chat) - end + context 'when instance level setting already exists' do + let_it_be(:instance_setting) do + create(:instance_model_selection_feature_setting, feature: :duo_chat) + end - it 'returns the existing instance level setting' do - expect(response).to be_success - expect(response.payload).to eq(instance_setting) - end + it 'returns the existing instance level setting' do + expect(response).to be_success + expect(response.payload).to eq(instance_setting) end + end - context 'when both self-hosted and instance settings exist' do - let_it_be(:ai_feature_setting) do - create(:ai_feature_setting, self_hosted_model: self_hosted_model, feature: :duo_chat) - end + context 'when both self-hosted and instance settings exist' do + let_it_be(:ai_feature_setting) do + create(:ai_feature_setting, self_hosted_model: self_hosted_model, feature: :duo_chat) + end - let_it_be(:instance_setting) do - create(:instance_model_selection_feature_setting, feature: :duo_chat) - end + let_it_be(:instance_setting) do + create(:instance_model_selection_feature_setting, feature: :duo_chat) + end - it 'prioritizes self-hosted feature setting over instance setting' do - expect(response).to be_success - expect(response.payload).to eq(ai_feature_setting) - end + it 'prioritizes self-hosted feature setting over instance setting' do + expect(response).to be_success + expect(response.payload).to eq(ai_feature_setting) end end end diff --git a/ee/spec/services/ai/model_selection/update_self_managed_model_selection_service_spec.rb b/ee/spec/services/ai/model_selection/update_self_managed_model_selection_service_spec.rb index 04a2a1b6280268..0aca1120c96d9a 100644 --- a/ee/spec/services/ai/model_selection/update_self_managed_model_selection_service_spec.rb +++ b/ee/spec/services/ai/model_selection/update_self_managed_model_selection_service_spec.rb @@ -49,10 +49,8 @@ end describe '#execute' do - context 'when instance_level_model_selection feature flag is disabled' do - before do - stub_feature_flags(instance_level_model_selection: false) - end + context 'when provider is not vendored' do + let(:provider) { "self_hosted" } it 'only calls the feature setting update service' do expect(Ai::ModelSelection::UpsertInstanceFeatureSettingService).not_to receive(:new) @@ -60,89 +58,71 @@ expect(response).to be_success expect(response.payload).to be_an(Ai::FeatureSetting) expect(response.payload.feature).to eq(feature.to_s) + expect(response.payload.provider).to eq('self_hosted') end end - context 'when instance_level_model_selection feature flag is enabled' do - before do - stub_feature_flags(instance_level_model_selection: true) - end - - context 'when provider is not vendored' do - let(:provider) { "self_hosted" } - - it 'only calls the feature setting update service' do - expect(Ai::ModelSelection::UpsertInstanceFeatureSettingService).not_to receive(:new) + context 'when provider is vendored' do + let(:provider) { "vendored" } + let(:instance_feature_setting) { create(:instance_model_selection_feature_setting, feature: feature) } + + it 'calls both instance feature setting service and feature setting update service' do + expect(Ai::ModelSelection::UpsertInstanceFeatureSettingService).to receive(:new).with( + user, + feature, + offered_model_ref + ).and_call_original + expect(Ai::FeatureSettings::UpdateService).to receive(:new).with( + an_instance_of(Ai::FeatureSetting), + user, + { + feature: feature, + provider: provider, + ai_self_hosted_model_id: nil + }).and_call_original - expect(response).to be_success - expect(response.payload).to be_an(Ai::FeatureSetting) - expect(response.payload.feature).to eq(feature.to_s) - expect(response.payload.provider).to eq('self_hosted') - end + expect(response).to be_success + expect(response.payload).to be_an(Ai::FeatureSetting) + expect(response.payload.feature).to eq(feature.to_s) + expect(response.payload.provider).to eq(provider.to_s) end - context 'when provider is vendored' do - let(:provider) { "vendored" } - let(:instance_feature_setting) { create(:instance_model_selection_feature_setting, feature: feature) } - - it 'calls both instance feature setting service and feature setting update service' do - expect(Ai::ModelSelection::UpsertInstanceFeatureSettingService).to receive(:new).with( - user, - feature, - offered_model_ref - ).and_call_original - expect(Ai::FeatureSettings::UpdateService).to receive(:new).with( - an_instance_of(Ai::FeatureSetting), - user, - { - feature: feature, - provider: provider, - ai_self_hosted_model_id: nil - }).and_call_original - - expect(response).to be_success - expect(response.payload).to be_an(Ai::FeatureSetting) - expect(response.payload.feature).to eq(feature.to_s) - expect(response.payload.provider).to eq(provider.to_s) + context 'when instance feature setting service returns error' do + let(:params) do + { + feature: feature, + provider: provider, + offered_model_ref: 'invalid', + ai_self_hosted_model_id: nil + } end - context 'when instance feature setting service returns error' do - let(:params) do - { - feature: feature, - provider: provider, - offered_model_ref: 'invalid', - ai_self_hosted_model_id: nil - } - end + it 'returns the error from instance service without calling feature setting service' do + expect(Ai::FeatureSettings::UpdateService).not_to receive(:new) - it 'returns the error from instance service without calling feature setting service' do - expect(Ai::FeatureSettings::UpdateService).not_to receive(:new) + expect(response).to be_error + expect(response.payload).to be_an(::Ai::ModelSelection::InstanceModelSelectionFeatureSetting) + expect(response.message).to include 'Offered model ref Feature not found in model definitions' + end + end - expect(response).to be_error - expect(response.payload).to be_an(::Ai::ModelSelection::InstanceModelSelectionFeatureSetting) - expect(response.message).to include 'Offered model ref Feature not found in model definitions' - end + context 'when instance feature setting service succeeds but feature setting service fails' do + let(:params) do + { + feature: feature, + provider: "vendored", + offered_model_ref: nil, + ai_self_hosted_model_id: 'invalid_id' + } end - context 'when instance feature setting service succeeds but feature setting service fails' do - let(:params) do - { - feature: feature, - provider: "vendored", - offered_model_ref: nil, - ai_self_hosted_model_id: 'invalid_id' - } + it 'returns the error from feature setting service' do + allow_next_instance_of(::Ai::FeatureSettings::UpdateService) do |service| + allow(service).to receive(:execute).and_return(ServiceResponse.error(message: "Something went wrong")) end - it 'returns the error from feature setting service' do - allow_next_instance_of(::Ai::FeatureSettings::UpdateService) do |service| - allow(service).to receive(:execute).and_return(ServiceResponse.error(message: "Something went wrong")) - end - - expect(response).to be_error - expect(response.message).to eq("Something went wrong") - end + expect(response).to be_error + expect(response.message).to eq("Something went wrong") end end end -- GitLab From 7417a3d7e0e9dd1e13cd44b67f5137c7e502a1f5 Mon Sep 17 00:00:00 2001 From: Cindy Halim Date: Tue, 21 Oct 2025 15:40:09 -0400 Subject: [PATCH 2/3] Remove flag in duo_agent_platform_model_metadata_service --- ...o_agent_platform_model_metadata_service.rb | 21 +++-- ...nt_platform_model_metadata_service_spec.rb | 86 +++++++------------ 2 files changed, 46 insertions(+), 61 deletions(-) diff --git a/ee/app/services/ai/duo_workflows/duo_agent_platform_model_metadata_service.rb b/ee/app/services/ai/duo_workflows/duo_agent_platform_model_metadata_service.rb index e14ab0cdeed31d..dac21801963bf1 100644 --- a/ee/app/services/ai/duo_workflows/duo_agent_platform_model_metadata_service.rb +++ b/ee/app/services/ai/duo_workflows/duo_agent_platform_model_metadata_service.rb @@ -25,16 +25,21 @@ def self_managed? !::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) end + def duo_agent_platform + ::Ai::FeatureSetting.duo_agent_platform + end + def duo_agent_platform_in_self_hosted_duo? - ::Ai::FeatureSetting.duo_agent_platform.self_hosted.exists? + # Disabled only exists in in Duo Self-Hosted context + return true if duo_agent_platform.disabled.exists? + + duo_agent_platform.self_hosted.exists? end def resolve_self_managed_model_metadata - if duo_agent_platform_in_self_hosted_duo? - resolve_self_hosted_duo_model_metadata - else - resolve_cloud_connected_model_metadata - end + return resolve_self_hosted_duo_model_metadata if duo_agent_platform_in_self_hosted_duo? + + resolve_cloud_connected_model_metadata end # Self-Hosted Duo Priority: @@ -43,6 +48,8 @@ def resolve_self_managed_model_metadata def resolve_self_hosted_duo_model_metadata return {} unless Feature.enabled?(:self_hosted_agent_platform, :instance) + return {} if duo_agent_platform.disabled.exists? + feature_setting = ::Ai::FeatureSetting.find_by_feature(FEATURE_NAME) model_metadata_from_setting(feature_setting) @@ -52,8 +59,6 @@ def resolve_self_hosted_duo_model_metadata # 1. Instance-level model selection (admin sets instance defaults) # 2. User model selection (users can override instance defaults) def resolve_cloud_connected_model_metadata - return {} unless Feature.enabled?(:instance_level_model_selection, :instance) - # Priority 1: Instance-level model selection instance_setting = ::Ai::ModelSelection::InstanceModelSelectionFeatureSetting .find_or_initialize_by_feature(FEATURE_NAME) diff --git a/ee/spec/services/ai/duo_workflows/duo_agent_platform_model_metadata_service_spec.rb b/ee/spec/services/ai/duo_workflows/duo_agent_platform_model_metadata_service_spec.rb index 46d822038ec984..505a8b671aa058 100644 --- a/ee/spec/services/ai/duo_workflows/duo_agent_platform_model_metadata_service_spec.rb +++ b/ee/spec/services/ai/duo_workflows/duo_agent_platform_model_metadata_service_spec.rb @@ -42,7 +42,6 @@ context 'when self_hosted_agent_platform feature flag is enabled' do before do stub_feature_flags(self_hosted_agent_platform: true) - stub_feature_flags(instance_level_model_selection: false) stub_feature_flags(duo_agent_platform_model_selection: false) end @@ -84,67 +83,50 @@ context 'for cloud-connected self-managed instances' do let(:root_namespace) { nil } - context 'when instance_level_model_selection feature flag is enabled' do - before do - stub_feature_flags(instance_level_model_selection: true) - stub_feature_flags(duo_agent_platform_model_selection: false) - stub_feature_flags(self_hosted_agent_platform: false) - end - - context 'with instance model selection setting (priority 1)' do - let!(:instance_setting) do - create(:instance_model_selection_feature_setting, - feature: :duo_agent_platform, - offered_model_ref: 'claude-3-7-sonnet-20250219' - ) - end - - it 'returns model metadata headers for cloud-connected' do - result = service.execute + before do + stub_feature_flags(duo_agent_platform_model_selection: false) + stub_feature_flags(self_hosted_agent_platform: false) + end - expect(result).to eq( - 'x-gitlab-agent-platform-model-metadata' => { - 'provider' => 'gitlab', - 'feature_setting' => 'duo_agent_platform', - 'identifier' => 'claude-3-7-sonnet-20250219' - }.to_json - ) - end + context 'with instance model selection setting (priority 1)' do + let!(:instance_setting) do + create(:instance_model_selection_feature_setting, + feature: :duo_agent_platform, + offered_model_ref: 'claude-3-7-sonnet-20250219' + ) end - context 'when instance setting has no model ref' do - let!(:instance_setting) do - create(:instance_model_selection_feature_setting, - feature: :duo_agent_platform, - offered_model_ref: nil - ) - end - - it 'returns model metadata headers for cloud-connected wit gitlab default model' do - result = service.execute + it 'returns model metadata headers for cloud-connected' do + result = service.execute - expect(result).to eq( - 'x-gitlab-agent-platform-model-metadata' => { - 'provider' => 'gitlab', - 'feature_setting' => 'duo_agent_platform', - 'identifier' => nil - }.to_json - ) - end + expect(result).to eq( + 'x-gitlab-agent-platform-model-metadata' => { + 'provider' => 'gitlab', + 'feature_setting' => 'duo_agent_platform', + 'identifier' => 'claude-3-7-sonnet-20250219' + }.to_json + ) end end - context 'when instance_level_model_selection feature flag is disabled' do - before do - stub_feature_flags(instance_level_model_selection: false) - stub_feature_flags(duo_agent_platform_model_selection: false) - stub_feature_flags(self_hosted_agent_platform: false) + context 'when instance setting has no model ref' do + let!(:instance_setting) do + create(:instance_model_selection_feature_setting, + feature: :duo_agent_platform, + offered_model_ref: nil + ) end - it 'returns empty headers' do + it 'returns model metadata headers for cloud-connected wit gitlab default model' do result = service.execute - expect(result).to eq({}) + expect(result).to eq( + 'x-gitlab-agent-platform-model-metadata' => { + 'provider' => 'gitlab', + 'feature_setting' => 'duo_agent_platform', + 'identifier' => nil + }.to_json + ) end end end @@ -156,7 +138,6 @@ before do stub_feature_flags(duo_agent_platform_model_selection: group) stub_feature_flags(ai_model_switching: group) - stub_feature_flags(instance_level_model_selection: false) stub_feature_flags(self_hosted_agent_platform: false) end @@ -335,7 +316,6 @@ before do stub_feature_flags(duo_agent_platform_model_selection: false) stub_feature_flags(ai_model_switching: false) - stub_feature_flags(instance_level_model_selection: false) stub_feature_flags(self_hosted_agent_platform: false) end -- GitLab From f4da5c620201d834e883e6af9e0eedbc7998e227 Mon Sep 17 00:00:00 2001 From: Cindy Halim Date: Thu, 23 Oct 2025 18:06:29 -0400 Subject: [PATCH 3/3] Remove flag in tasks/code_completion specs --- .../code_suggestions/tasks/code_completion.rb | 5 +- .../tasks/code_completion_spec.rb | 115 +++++++----------- 2 files changed, 47 insertions(+), 73 deletions(-) diff --git a/ee/lib/code_suggestions/tasks/code_completion.rb b/ee/lib/code_suggestions/tasks/code_completion.rb index eb835d1867b157..e029e10d92f1f3 100644 --- a/ee/lib/code_suggestions/tasks/code_completion.rb +++ b/ee/lib/code_suggestions/tasks/code_completion.rb @@ -68,10 +68,7 @@ def model_switching_ai_gateway_prompt end def self_hosted_vendored_prompt - if Feature.enabled?(:instance_level_model_selection, :instance) && - feature_setting.set_to_gitlab_default? - return saas_prompt - end + return saas_prompt if feature_setting.set_to_gitlab_default? CodeSuggestions::Prompts::CodeCompletion::SelfHosted::Vendored.new( params, diff --git a/ee/spec/lib/code_suggestions/tasks/code_completion_spec.rb b/ee/spec/lib/code_suggestions/tasks/code_completion_spec.rb index 36443bc29fa45f..7766ba966d5a5e 100644 --- a/ee/spec/lib/code_suggestions/tasks/code_completion_spec.rb +++ b/ee/spec/lib/code_suggestions/tasks/code_completion_spec.rb @@ -90,8 +90,7 @@ stub_feature_flags( incident_fail_over_completion_provider: false, use_claude_code_completion: false, - code_completion_opt_out_fireworks: false, - instance_level_model_selection: false + code_completion_opt_out_fireworks: false ) end @@ -167,6 +166,7 @@ context 'on GitLab saas' do before do allow(Gitlab).to receive(:org_or_com?).and_return(true) + stub_saas_features(gitlab_com_subscriptions: true) end let_it_be(:group1) do @@ -499,84 +499,61 @@ create(:ai_feature_setting, :code_completions, provider: :vendored) end - it_behaves_like 'code suggestion task' do - let(:expected_body) do - { - "current_file" => { - "file_name" => "test.py", - "content_above_cursor" => "sor", - "content_below_cursor" => "som" - }, - "telemetry" => [], - "stream" => false, - "model_provider" => "gitlab", - "model_name" => "" - } - end - - let(:expected_feature_name) { :code_suggestions } + let(:model_details) do + { + "model_provider" => "gitlab", + "model_name" => "" + } end - context 'when :instance_level_model_selection is enabled' do - let(:model_details) do - { - "model_provider" => "gitlab", - "model_name" => "" - } - end + let(:expected_body) do + { + "current_file" => { + "file_name" => "test.py", + "content_above_cursor" => "sor", + "content_below_cursor" => "som" + }, + "telemetry" => [], + "stream" => false, + **model_details + } + end - let(:expected_body) do - { - "current_file" => { - "file_name" => "test.py", - "content_above_cursor" => "sor", - "content_below_cursor" => "som" - }, - "telemetry" => [], - "stream" => false, - **model_details - } - end + let(:expected_feature_name) { :code_suggestions } - let(:expected_feature_name) { :code_suggestions } + before do + allow(Gitlab).to receive(:com?).and_return(false) + end - before do - stub_feature_flags(instance_level_model_selection: true) - allow(Gitlab).to receive(:com?).and_return(false) + context 'and default model at the instance level' do + it_behaves_like 'code suggestion task' do + let(:model_details) do + { + "model_name" => "codestral-2501", + "model_provider" => "fireworks_ai", + "prompt_version" => 1 + } + end - ai_feature_setting.update!(self_hosted_model: nil, provider: :vendored) + let(:expected_feature_name) { :code_suggestions } end + end - context 'and instance level is default' do - it_behaves_like 'code suggestion task' do - let(:model_details) do - { - "model_name" => "codestral-2501", - "model_provider" => "fireworks_ai", - "prompt_version" => 1 - } - end - - let(:expected_feature_name) { :code_suggestions } + context 'and vendored model is pinned' do + it_behaves_like 'code suggestion task' do + before do + create(:instance_model_selection_feature_setting, feature: :code_completions, + offered_model_ref: "claude_sonnet_3_5") end - end - context 'and vendored model is pinned' do - it_behaves_like 'code suggestion task' do - before do - create(:instance_model_selection_feature_setting, feature: :code_completions, - offered_model_ref: "claude_sonnet_3_5") - end - - let(:model_details) do - { - "model_name" => "claude_sonnet_3_5", - "model_provider" => "gitlab" - } - end - - let(:expected_feature_name) { :code_suggestions } + let(:model_details) do + { + "model_name" => "claude_sonnet_3_5", + "model_provider" => "gitlab" + } end + + let(:expected_feature_name) { :code_suggestions } end end end -- GitLab