akadmin/chatwoot
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat: Harden model override preferences (5/6) (#14846)

Browse Source 
## Description

Hardens the Captain model override preferences API so account-level
overrides follow the same feature-router contract used by runtime LLM
calls. The API now permits model and feature keys from `llm.yml`,
removes blank model overrides, rejects invalid saved model combinations,
and returns each feature's effective model, provider, and source for UI
clients.

Fixes https://linear.app/chatwoot/issue/CW-7425/test-new-models

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality not to work as expected)
- [ ] This change requires a documentation update

## How Has This Been Tested?

Verified the account preferences API and account model validation
behavior for valid overrides, invalid model values, unknown feature
keys, blank override removal, and effective model/provider/source
payload metadata.

- `eval "$(rbenv init -)" && bundle exec rspec
spec/controllers/api/v1/accounts/captain/preferences_controller_spec.rb
spec/models/account_spec.rb
spec/models/concerns/captain_featurable_spec.rb
spec/lib/llm/feature_router_spec.rb`
- `eval "$(rbenv init -)" && bundle exec rubocop
app/controllers/api/v1/accounts/captain/preferences_controller.rb
app/models/concerns/account_settings_schema.rb
app/models/concerns/captain_featurable.rb
spec/controllers/api/v1/accounts/captain/preferences_controller_spec.rb
spec/models/account_spec.rb
spec/models/concerns/captain_featurable_spec.rb
spec/lib/llm/feature_router_spec.rb`
- `git diff --check`

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [x] I have commented on my code, particularly in hard-to-understand
areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream
modules
This commit is contained in:
Sony Mathew
2026-06-25 17:38:11 +05:30
committed by GitHub
parent 4e26c5b4bb
commit b8b62ad0f1
11 changed files with 344 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/controllers/api/v1/accounts/captain/preferences_controller.rb
+8 -5
View File
@@ -8,8 +8,8 @@ class Api::V1::Accounts::Captain::PreferencesController < Api::V1::Accounts::Bas
def update
params_to_update = captain_params
@current_account.captain_models = params_to_update[:captain_models] if params_to_update[:captain_models]
@current_account.captain_features = params_to_update[:captain_features] if params_to_update[:captain_features]
@current_account.captain_models = params_to_update[:captain_models] if params_to_update.key?(:captain_models)
@current_account.captain_features = params_to_update[:captain_features] if params_to_update.key?(:captain_features)
@current_account.save!
render json: preferences_payload
@@ -38,7 +38,7 @@ class Api::V1::Accounts::Captain::PreferencesController < Api::V1::Accounts::Bas
def merged_captain_models
existing_models = @current_account.captain_models || {}
existing_models.merge(permitted_captain_models)
existing_models.merge(permitted_captain_models).compact_blank.presence
end
def merged_captain_features
@@ -61,13 +61,16 @@ class Api::V1::Accounts::Captain::PreferencesController < Api::V1::Accounts::Bas
def features_with_account_preferences
preferences = Current.account.captain_preferences
account_features = preferences[:features] || {}
account_models = preferences[:models] || {}
Llm::Models.feature_keys.index_with do |feature_key|
config = Llm::Models.feature_config(feature_key)
route = Llm::FeatureRouter.resolve(feature: feature_key, account: Current.account)
config.merge(
enabled: account_features[feature_key] == true,
selected: account_models[feature_key] || config[:default]
model: route[:model],
selected: route[:model],
provider: route[:provider],
source: route[:source]
)
end
end
app/controllers/super_admin/accounts_controller.rb
+2 -1
View File
@@ -35,7 +35,8 @@ class SuperAdmin::AccountsController < SuperAdmin::ApplicationController
#
def resource_params
permitted_params = super
permitted_params[:limits] = permitted_params[:limits].to_h.compact
permitted_params[:limits] = permitted_params[:limits].to_h.compact if permitted_params.key?(:limits)
permitted_params[:captain_models] = permitted_params[:captain_models].to_h.compact_blank.presence if permitted_params.key?(:captain_models)
permitted_params[:selected_feature_flags] = params[:enabled_features].keys.map(&:to_sym) if params[:enabled_features].present?
permitted_params
end
app/dashboards/account_dashboard.rb
+4 -1
View File
@@ -18,6 +18,7 @@ class AccountDashboard < Administrate::BaseDashboard
# Add all_features last so it appears after manually_managed_features
attributes[:all_features] = AccountFeaturesField
attributes[:captain_models] = CaptainModelOverridesField
attributes
else
@@ -57,6 +58,7 @@ class AccountDashboard < Administrate::BaseDashboard
attrs = %i[custom_attributes limits]
attrs << :manually_managed_features if ChatwootApp.chatwoot_cloud?
attrs << :all_features
attrs << :captain_models
attrs
else
[]
@@ -79,6 +81,7 @@ class AccountDashboard < Administrate::BaseDashboard
attrs = %i[limits]
attrs << :manually_managed_features if ChatwootApp.chatwoot_cloud?
attrs << :all_features
attrs << :captain_models
attrs
else
[]
@@ -117,7 +120,7 @@ class AccountDashboard < Administrate::BaseDashboard
# to prevent an error from being raised (wrong number of arguments)
# Reference: https://github.com/thoughtbot/administrate/pull/2356/files#diff-4e220b661b88f9a19ac527c50d6f1577ef6ab7b0bed2bfdf048e22e6bfa74a05R204
def permitted_attributes(action)
attrs = super + [limits: {}]
attrs = super + [limits: {}, captain_models: {}]
# Add manually_managed_features to permitted attributes only for Chatwoot Cloud
attrs << { manually_managed_features: [] } if ChatwootApp.chatwoot_cloud?
app/models/concerns/captain_featurable.rb
+18 -1
View File
@@ -4,6 +4,7 @@ module CaptainFeaturable
extend ActiveSupport::Concern
included do
before_validation :normalize_captain_models
validate :validate_captain_models
# Dynamically define accessor methods for each captain feature
@@ -46,11 +47,27 @@ module CaptainFeaturable
return if captain_models.blank?
captain_models.each do |feature_key, model_name|
next if model_name.blank?
unless Llm::Models.feature?(feature_key)
errors.add(:captain_models, "'#{feature_key}' is not a known feature")
next
end
next if Llm::Models.valid_model_for?(feature_key, model_name)
allowed_models = Llm::Models.models_for(feature_key)
errors.add(:captain_models, "'#{model_name}' is not a valid model for #{feature_key}. Allowed: #{allowed_models.join(', ')}")
end
end
def normalize_captain_models
return unless captain_models.is_a?(Hash)
normalized_models = captain_models.each_with_object({}) do |(feature_key, model_name), result|
next if model_name.blank?
result[feature_key.to_s] = model_name.to_s
end
self.captain_models = normalized_models.presence
end
end
config/locales/en.yml
+22
View File
@@ -574,6 +574,28 @@ en:
ssl_status:
custom_domain_not_configured: 'Custom domain is not configured'
super_admin:
captain_model_overrides:
form:
helper_text: 'Leave a model blank to use the YAML default for that AI feature.'
use_default: 'Use default: %{model} (%{model_id})'
show:
summary: 'View model routing'
provider: 'Provider'
model: 'Model'
sources:
account_override: 'Account override'
default: 'Default'
features:
editor: 'Editor'
assistant: 'Assistant'
copilot: 'Copilot'
label_suggestion: 'Label suggestion'
document_faq_generation: 'Document FAQ generation'
help_center_article_generation: 'Help center article generation'
onboarding_content_generation: 'Onboarding content generation'
help_center_query_translation: 'Help center query translation'
audio_transcription: 'Audio transcription'
help_center_search: 'Help center search'
push_diagnostics:
user_not_found: 'User not found.'
no_subscriptions_to_test: 'Select at least one subscription to test.'
enterprise/app/fields/captain_model_overrides_field.rb
+56
View File
@@ -0,0 +1,56 @@
require 'administrate/field/base'
class CaptainModelOverridesField < Administrate::Field::Base
def feature_rows
Llm::Models.feature_keys.map do |feature_key|
route = Llm::FeatureRouter.resolve(feature: feature_key, account: resource)
{
key: feature_key,
name: feature_name(feature_key),
provider: provider_label(route[:provider]),
provider_id: route[:provider],
model: model_label(route[:model]),
model_id: route[:model],
default_model: model_label(default_model_id(feature_key)),
default_model_id: default_model_id(feature_key),
source: route[:source],
source_label: source_label(route[:source]),
selected_override: selected_override(feature_key),
options: model_options(feature_key)
}
end
end
private
def selected_override(feature_key)
resource.captain_models&.[](feature_key).presence
end
def default_model_id(feature_key)
Llm::Models.default_model_for(feature_key)
end
def model_options(feature_key)
Llm::Models.feature_config(feature_key)[:models].map do |model|
[model[:display_name] || model[:id], model[:id]]
end
end
def model_label(model_id)
Llm::Models.model_config(model_id)&.dig('display_name') || model_id
end
def provider_label(provider_id)
Llm::Models.providers.dig(provider_id, 'display_name') || provider_id
end
def feature_name(feature_key)
I18n.t("super_admin.captain_model_overrides.features.#{feature_key}", default: feature_key.humanize)
end
def source_label(source)
I18n.t("super_admin.captain_model_overrides.sources.#{source}")
end
end
enterprise/app/views/fields/captain_model_overrides_field/_form.html.erb
+27
View File
@@ -0,0 +1,27 @@
<div class="field-unit__label">
<%= f.label field.attribute %>
</div>
<div class="field-unit__field">
<p class="text-gray-400 text-xs italic mb-4"><%= t('super_admin.captain_model_overrides.form.helper_text') %></p>
<div class="grid grid-cols-1 gap-4">
<% field.feature_rows.each do |feature| %>
<div class="p-3 bg-white rounded-lg shadow-sm outline outline-1 outline-n-container">
<div class="mb-2">
<div class="text-sm font-medium text-slate-700"><%= feature[:name] %></div>
<div class="text-xs text-slate-500"><%= feature[:key] %></div>
</div>
<%= select_tag(
"account[captain_models][#{feature[:key]}]",
options_for_select(
[[t('super_admin.captain_model_overrides.form.use_default', model: feature[:default_model], model_id: feature[:default_model_id]), '']] + feature[:options],
feature[:selected_override]
),
class: 'block w-full rounded-md border-slate-300 text-sm'
) %>
</div>
<% end %>
</div>
</div>
enterprise/app/views/fields/captain_model_overrides_field/_show.html.erb
+43
View File
@@ -0,0 +1,43 @@
<details class="group w-full">
<summary class="flex cursor-pointer list-none items-center gap-2 text-sm font-medium text-n-slate-12">
<span><%= t('super_admin.captain_model_overrides.show.summary') %></span>
<svg class="h-4 w-4 transition-transform duration-200 group-open:rotate-180" viewBox="0 0 20 20" fill="currentColor">
<path fill-rule="evenodd" d="M5.293 7.293a1 1 0 011.414 0L10 10.586l3.293-3.293a1 1 0 111.414 1.414l-4 4a1 1 0 01-1.414 0l-4-4a1 1 0 010-1.414z" clip-rule="evenodd" />
</svg>
</summary>
<div class="mt-3 w-full">
<div class="grid grid-cols-1 gap-3">
<% field.feature_rows.each do |feature| %>
<div class="p-3 bg-white rounded-md outline outline-n-container outline-1 shadow-sm">
<div class="flex items-center justify-between gap-4">
<div>
<div class="text-sm font-medium text-n-slate-12"><%= feature[:name] %></div>
<div class="text-xs text-n-slate-11"><%= feature[:key] %></div>
</div>
<span class="<%= feature[:source] == :account_override ? 'bg-green-400 text-white' : 'bg-slate-50 text-slate-800' %> rounded-full px-2 py-1 text-xs">
<%= feature[:source_label] %>
</span>
</div>
<div class="mt-3 grid grid-cols-1 md:grid-cols-2 gap-3 text-sm">
<div>
<div class="text-xs uppercase text-n-slate-10"><%= t('super_admin.captain_model_overrides.show.provider') %></div>
<div class="text-n-slate-12">
<%= feature[:provider] %>
<span class="text-n-slate-10">(<%= feature[:provider_id] %>)</span>
</div>
</div>
<div>
<div class="text-xs uppercase text-n-slate-10"><%= t('super_admin.captain_model_overrides.show.model') %></div>
<div class="text-n-slate-12">
<%= feature[:model] %>
<span class="text-n-slate-10">(<%= feature[:model_id] %>)</span>
</div>
</div>
</div>
</div>
<% end %>
</div>
</div>
</details>
spec/controllers/api/v1/accounts/captain/preferences_controller_spec.rb
+59
View File
@@ -45,6 +45,28 @@ RSpec.describe 'Api::V1::Accounts::Captain::Preferences', type: :request do
expect(json_response).to have_key(:models)
expect(json_response).to have_key(:features)
end
it 'returns effective model provider and source for each feature' do
account.update!(captain_models: { 'editor' => 'gpt-4.1' })
get "/api/v1/accounts/#{account.id}/captain/preferences",
headers: admin.create_new_auth_token,
as: :json
expect(response).to have_http_status(:success)
expect(json_response.dig(:features, :editor)).to include(
model: 'gpt-4.1',
selected: 'gpt-4.1',
provider: 'openai',
source: 'account_override'
)
expect(json_response.dig(:features, :label_suggestion)).to include(
model: Llm::Models.default_model_for('label_suggestion'),
selected: Llm::Models.default_model_for('label_suggestion'),
provider: 'openai',
source: 'default'
)
end
end
end
@@ -84,6 +106,43 @@ RSpec.describe 'Api::V1::Accounts::Captain::Preferences', type: :request do
expect(account.reload.captain_models['editor']).to eq('gpt-4.1-mini')
end
it 'does not persist unknown captain model feature keys' do
put "/api/v1/accounts/#{account.id}/captain/preferences",
headers: admin.create_new_auth_token,
params: { captain_models: { editor: 'gpt-4.1-mini', unknown_feature: 'gpt-4.1' } },
as: :json
expect(response).to have_http_status(:success)
expect(account.reload.captain_models).to eq('editor' => 'gpt-4.1-mini')
end
it 'rejects invalid captain model values for the feature' do
put "/api/v1/accounts/#{account.id}/captain/preferences",
headers: admin.create_new_auth_token,
params: { captain_models: { label_suggestion: 'gpt-5.1' } },
as: :json
expect(response).to have_http_status(:unprocessable_entity)
expect(json_response[:message]).to include('not a valid model for label_suggestion')
expect(account.reload.captain_models).to be_nil
end
it 'removes blank captain model overrides' do
account.update!(captain_models: { 'editor' => 'gpt-4.1' })
put "/api/v1/accounts/#{account.id}/captain/preferences",
headers: admin.create_new_auth_token,
params: { captain_models: { editor: '' } },
as: :json
expect(response).to have_http_status(:success)
expect(account.reload.captain_models).to be_nil
expect(json_response.dig(:features, :editor)).to include(
selected: Llm::Models.default_model_for('editor'),
source: 'default'
)
end
it 'updates captain_models for document FAQ generation' do
put "/api/v1/accounts/#{account.id}/captain/preferences",
headers: admin.create_new_auth_token,
spec/controllers/super_admin/accounts_controller_spec.rb
+92
View File
@@ -25,6 +25,98 @@ RSpec.describe 'Super Admin accounts API', type: :request do
end
end
describe 'GET /super_admin/accounts/{account_id}' do
context 'when it is an authenticated user' do
it 'shows effective Captain model routing', if: ChatwootApp.enterprise? do
account.update!(captain_models: { 'editor' => 'gpt-4.1' })
sign_in(super_admin, scope: :super_admin)
get "/super_admin/accounts/#{account.id}"
document = Nokogiri::HTML(response.body)
summaries = document.css('details summary').map { |summary| summary.text.squish }
expect(response).to have_http_status(:success)
expect(document.at_css('#captain_models').text.squish).to eq('Captain models')
expect(summaries).to include('View model routing')
expect(summaries).not_to include('All features')
expect(summaries).not_to include('Captain models')
expect(response.body).to include('Editor', 'OpenAI', 'openai', 'gpt-4.1', 'Account override', 'Label suggestion', 'Default')
end
end
end
describe 'GET /super_admin/accounts/{account_id}/edit' do
context 'when it is an authenticated user' do
it 'renders a Captain model selector for every AI feature', if: ChatwootApp.enterprise? do
account.update!(captain_models: { 'editor' => 'gpt-4.1' })
sign_in(super_admin, scope: :super_admin)
get "/super_admin/accounts/#{account.id}/edit"
expect(response).to have_http_status(:success)
Llm::Models.feature_keys.each do |feature_key|
expect(response.body).to include("account[captain_models][#{feature_key}]")
end
document = Nokogiri::HTML(response.body)
editor_select = document.at_css('select[name="account[captain_models][editor]"]')
default_model_id = Llm::Models.default_model_for('editor')
default_model = Llm::Models.model_config(default_model_id)['display_name']
expect(editor_select.at_css('option[value=""]').text.squish).to eq("Use default: #{default_model} (#{default_model_id})")
end
end
end
describe 'PATCH /super_admin/accounts/{account_id}' do
context 'when it is an authenticated user' do
it 'updates Captain model overrides without changing unrelated settings' do
account.update!(
captain_models: { 'editor' => 'gpt-4.1' },
keep_pending_on_bot_failure: true
)
sign_in(super_admin, scope: :super_admin)
patch "/super_admin/accounts/#{account.id}",
params: {
account: {
name: account.name,
locale: account.locale,
status: account.status,
captain_models: {
editor: '',
assistant: 'gpt-5.2'
}
}
}
expect(response).to have_http_status(:redirect)
expect(account.reload.captain_models).to eq('assistant' => 'gpt-5.2')
expect(account.keep_pending_on_bot_failure).to be true
end
it 'rejects invalid Captain model overrides' do
sign_in(super_admin, scope: :super_admin)
patch "/super_admin/accounts/#{account.id}",
params: {
account: {
name: account.name,
locale: account.locale,
status: account.status,
captain_models: {
label_suggestion: 'gpt-5.1'
}
}
}
expect(response).to have_http_status(:unprocessable_entity)
expect(response.body).to include('not a valid model for label_suggestion')
expect(account.reload.captain_models).to be_nil
end
end
end
describe 'POST /super_admin/accounts/{account_id}/reset_cache' do
before do
create(:label, account: account)
spec/models/account_spec.rb
+13
View File
@@ -385,6 +385,19 @@ RSpec.describe Account do
expect(account).to be_valid
end
it 'rejects unknown feature keys' do
account.captain_models = { 'unknown_feature' => 'gpt-4.1' }
expect(account).not_to be_valid
expect(account.errors[:captain_models]).to include("'unknown_feature' is not a known feature")
end
it 'removes blank model overrides before saving' do
account.update!(captain_models: { 'editor' => '', 'assistant' => 'gpt-5.2' })
expect(account.captain_models).to eq('assistant' => 'gpt-5.2')
end
end
end
end