Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Commit e2afbc2

Browse files
authored
FIX: correctly handle provider edit (#1125)
Prior to this commit, editing the provider wouldn't recompute the provider params. It would also not correctly recompute the "canEditURL" property. To make possible this commit has: - made a fix in core: discourse/discourse#31329 - ensures the provider params are recomputed when provider is changed - made the check on `canEditURL` based on form state and not initial model value Tests have been added to confirm the expected behavior.
1 parent 35f1562 commit e2afbc2

File tree

2 files changed

+53
-15
lines changed

2 files changed

+53
-15
lines changed

assets/javascripts/discourse/components/ai-llm-editor-form.gjs

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import Component from "@glimmer/component";
22
import { cached, tracked } from "@glimmer/tracking";
33
import { concat, fn, get } from "@ember/helper";
4-
import { action, computed } from "@ember/object";
4+
import { action } from "@ember/object";
55
import { LinkTo } from "@ember/routing";
66
import { later } from "@ember/runloop";
77
import { service } from "@ember/service";
@@ -37,8 +37,6 @@ export default class AiLlmEditorForm extends Component {
3737

3838
const info = this.args.llms.resultSetMeta.presets.findBy("id", id);
3939
const modelInfo = info.models.findBy("name", modelName);
40-
const params =
41-
this.args.llms.resultSetMeta.provider_params[info.provider] ?? {};
4240

4341
return {
4442
max_prompt_tokens: modelInfo.tokens,
@@ -47,12 +45,7 @@ export default class AiLlmEditorForm extends Component {
4745
display_name: modelInfo.display_name,
4846
name: modelInfo.name,
4947
provider: info.provider,
50-
provider_params: Object.fromEntries(
51-
Object.entries(params).map(([k, v]) => [
52-
k,
53-
v?.type === "enum" ? v.default : null,
54-
])
55-
),
48+
provider_params: this.computeProviderParams(info.provider),
5649
};
5750
}
5851

@@ -103,11 +96,6 @@ export default class AiLlmEditorForm extends Component {
10396
return this.testRunning || this.testResult !== null;
10497
}
10598

106-
@computed("args.model.provider")
107-
get canEditURL() {
108-
return this.args.model.provider !== "aws_bedrock";
109-
}
110-
11199
get modulesUsingModel() {
112100
const usedBy = this.args.model.used_by?.filter((m) => m.type !== "ai_bot");
113101

@@ -140,6 +128,21 @@ export default class AiLlmEditorForm extends Component {
140128
return !this.args.model.isNew;
141129
}
142130

131+
computeProviderParams(provider) {
132+
const params = this.args.llms.resultSetMeta.provider_params[provider] ?? {};
133+
return Object.fromEntries(
134+
Object.entries(params).map(([k, v]) => [
135+
k,
136+
v?.type === "enum" ? v.default : null,
137+
])
138+
);
139+
}
140+
141+
@action
142+
canEditURL(provider) {
143+
return provider !== "aws_bedrock";
144+
}
145+
143146
@action
144147
openAddQuotaModal(addItemToCollection) {
145148
this.modal.show(AiLlmQuotaModal, {
@@ -220,6 +223,12 @@ export default class AiLlmEditorForm extends Component {
220223
}
221224
}
222225

226+
@action
227+
setProvider(provider, { set }) {
228+
set("provider_params", this.computeProviderParams(provider));
229+
set("provider", provider);
230+
}
231+
223232
@action
224233
delete() {
225234
return this.dialog.confirm({
@@ -287,6 +296,7 @@ export default class AiLlmEditorForm extends Component {
287296
@disabled={{this.seeded}}
288297
@format="large"
289298
@validation="required"
299+
@onSet={{this.setProvider}}
290300
as |field|
291301
>
292302
<field.Select as |select|>
@@ -299,7 +309,7 @@ export default class AiLlmEditorForm extends Component {
299309
</form.Field>
300310

301311
{{#unless this.seeded}}
302-
{{#if this.canEditURL}}
312+
{{#if (this.canEditURL data.provider)}}
303313
<form.Field
304314
@name="url"
305315
@title={{i18n "discourse_ai.llms.url"}}

spec/system/llms/ai_llm_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,34 @@
7171
expect(llm.user_id).not_to be_nil
7272
end
7373

74+
context "when changing the provider" do
75+
it "correctly changes the provider params" do
76+
visit "/admin/plugins/discourse-ai/ai-llms"
77+
find("[data-llm-id='none'] button").click()
78+
form.field("provider").select("vllm")
79+
80+
expect(form).to have_field_with_name("provider_params.disable_system_prompt")
81+
expect(form).to have_no_field_with_name("provider_params.disable_native_tools")
82+
83+
form.field("provider").select("open_router")
84+
85+
expect(form).to have_field_with_name("provider_params.disable_streaming")
86+
expect(form).to have_field_with_name("provider_params.disable_native_tools")
87+
end
88+
89+
it "updates if the url can be edited" do
90+
visit "/admin/plugins/discourse-ai/ai-llms"
91+
find("[data-llm-id='none'] button").click()
92+
form.field("provider").select("vllm")
93+
94+
expect(form).to have_field_with_name("url")
95+
96+
form.field("provider").select("aws_bedrock")
97+
98+
expect(form).to have_no_field_with_name("url")
99+
end
100+
end
101+
74102
context "with quotas" do
75103
fab!(:llm_model_1) { Fabricate(:llm_model, name: "claude-2") }
76104
fab!(:group_1) { Fabricate(:group) }

0 commit comments

Comments
 (0)