mirror of
https://github.com/discourse/discourse.git
synced 2026-08-04 10:23:17 -05:00
FIX: Don’t create AI Problem check trackers without a target LLM. (#35447)
We need to define #targets, or the problem check code will create a tracker without a target when the check passes. This bug causes the check to ignore the perform_every constraint.
This commit is contained in:
@@ -26,23 +26,19 @@ class ProblemCheck::AiCreditHardLimit < ProblemCheck
|
||||
|
||||
private
|
||||
|
||||
def targets
|
||||
LlmModel.joins(:llm_credit_allocation).where("llm_models.id < 0").pluck("llm_models.id")
|
||||
end
|
||||
|
||||
def hard_limit_problem(model, allocation)
|
||||
details = {
|
||||
override_data = {
|
||||
model_id: model.id,
|
||||
model_name: model.display_name,
|
||||
reset_date: format_reset_date(allocation.next_reset_at),
|
||||
url: "#{Discourse.base_path}/admin/plugins/discourse-ai/ai-llms",
|
||||
}
|
||||
|
||||
message = I18n.t("dashboard.problem.ai_credit_hard_limit", details)
|
||||
|
||||
Problem.new(
|
||||
message,
|
||||
priority: "high",
|
||||
identifier: "ai_credit_hard_limit",
|
||||
target: model.id,
|
||||
details:,
|
||||
)
|
||||
problem(model, override_data:)
|
||||
end
|
||||
|
||||
def format_reset_date(date)
|
||||
|
||||
@@ -28,8 +28,12 @@ class ProblemCheck::AiCreditSoftLimit < ProblemCheck
|
||||
|
||||
private
|
||||
|
||||
def targets
|
||||
LlmModel.joins(:llm_credit_allocation).where("llm_models.id < 0").pluck("llm_models.id")
|
||||
end
|
||||
|
||||
def soft_limit_problem(model, allocation)
|
||||
details = {
|
||||
override_data = {
|
||||
model_id: model.id,
|
||||
model_name: model.display_name,
|
||||
percentage_remaining: allocation.percentage_remaining.round,
|
||||
@@ -37,15 +41,7 @@ class ProblemCheck::AiCreditSoftLimit < ProblemCheck
|
||||
url: "#{Discourse.base_path}/admin/plugins/discourse-ai/ai-llms",
|
||||
}
|
||||
|
||||
message = I18n.t("dashboard.problem.ai_credit_soft_limit", details)
|
||||
|
||||
Problem.new(
|
||||
message,
|
||||
priority: "low",
|
||||
identifier: "ai_credit_soft_limit",
|
||||
target: model.id,
|
||||
details:,
|
||||
)
|
||||
problem(model, override_data:)
|
||||
end
|
||||
|
||||
def format_reset_date(date)
|
||||
|
||||
@@ -8,11 +8,17 @@ class ProblemCheck::AiLlmStatus < ProblemCheck
|
||||
self.max_blips = 2
|
||||
|
||||
def call
|
||||
return [] if !SiteSetting.discourse_ai_enabled
|
||||
|
||||
llm_errors
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def targets
|
||||
LlmModel.in_use.pluck(:id)
|
||||
end
|
||||
|
||||
def llm_errors
|
||||
return [] if !SiteSetting.discourse_ai_enabled
|
||||
LlmModel.in_use.find_each.filter_map do |model|
|
||||
@@ -43,22 +49,13 @@ class ProblemCheck::AiLlmStatus < ProblemCheck
|
||||
)
|
||||
end
|
||||
|
||||
details = {
|
||||
override_data = {
|
||||
model_id: model.id,
|
||||
model_name: model.display_name,
|
||||
error: parse_error_message(e.message),
|
||||
url: "#{Discourse.base_path}/admin/plugins/discourse-ai/ai-llms/#{model.id}/edit",
|
||||
}
|
||||
|
||||
message = I18n.t("dashboard.problem.ai_llm_status", details)
|
||||
|
||||
Problem.new(
|
||||
message,
|
||||
priority: "high",
|
||||
identifier: "ai_llm_status",
|
||||
target: model.id,
|
||||
details:,
|
||||
)
|
||||
problem(model, override_data:, details: { error: parse_error_message(e.message) })
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
# frozen_string_literal: true
|
||||
class RemoveAiProblemCheckTrackersWithoutTargets < ActiveRecord::Migration[8.0]
|
||||
def up
|
||||
tracker_identifiers = %w[ai_llm_status ai_credit_soft_limit ai_credit_hard_limit]
|
||||
no_target = "__NULL__"
|
||||
|
||||
DB.exec(<<~SQL, tracker_identifiers: tracker_identifiers, no_target: no_target)
|
||||
DELETE FROM problem_check_trackers
|
||||
WHERE identifier IN (:tracker_identifiers) AND target = :no_target
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,80 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
RSpec.describe Jobs::RunProblemCheck do
|
||||
subject(:run_check_job) { described_class.new }
|
||||
|
||||
describe "integration specs for AI-based problem checks" do
|
||||
before { enable_current_plugin }
|
||||
|
||||
context "when running AI LLM status checks" do
|
||||
let(:identifier) { :ai_llm_status }
|
||||
|
||||
let!(:llm_model) { assign_fake_provider_to(:ai_default_llm_model) }
|
||||
|
||||
before { SiteSetting.ai_summarization_enabled = true }
|
||||
|
||||
context "when everything is OK" do
|
||||
it "creates a problem check tracker that is targeting the tested model" do
|
||||
DiscourseAi::Completions::Llm.with_prepared_responses(["OK"]) do
|
||||
run_check_job.execute(check_identifier: identifier)
|
||||
end
|
||||
|
||||
created_trackers = ProblemCheckTracker.where(identifier: identifier)
|
||||
|
||||
expect(created_trackers.size).to eq(1)
|
||||
expect(created_trackers.last.target).to eq(llm_model.id.to_s)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when running AI Credit soft limits checks" do
|
||||
let(:identifier) { :ai_credit_soft_limit }
|
||||
|
||||
fab!(:llm_model) { Fabricate(:llm_model, id: -1) }
|
||||
|
||||
context "when we haven't reach the soft limit yet" do
|
||||
it "creates a problem check tracker that is targeting the tested model" do
|
||||
Fabricate(
|
||||
:llm_credit_allocation,
|
||||
llm_model: llm_model,
|
||||
monthly_credits: 1000,
|
||||
monthly_used: 700,
|
||||
soft_limit_percentage: 80,
|
||||
)
|
||||
|
||||
run_check_job.execute(check_identifier: identifier)
|
||||
|
||||
created_trackers = ProblemCheckTracker.where(identifier: identifier)
|
||||
|
||||
expect(created_trackers.size).to eq(1)
|
||||
expect(created_trackers.last.target).to eq(llm_model.id.to_s)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when running AI Credit hard limits checks" do
|
||||
let(:identifier) { :ai_credit_hard_limit }
|
||||
|
||||
fab!(:llm_model) { Fabricate(:llm_model, id: -1) }
|
||||
|
||||
context "when we haven't reach the soft limit yet" do
|
||||
it "creates a problem check tracker that is targeting the tested model" do
|
||||
Fabricate(
|
||||
:llm_credit_allocation,
|
||||
llm_model: llm_model,
|
||||
monthly_credits: 1000,
|
||||
monthly_used: 850,
|
||||
soft_limit_percentage: 80,
|
||||
)
|
||||
|
||||
run_check_job.execute(check_identifier: identifier)
|
||||
|
||||
created_trackers = ProblemCheckTracker.where(identifier: identifier)
|
||||
|
||||
expect(created_trackers.size).to eq(1)
|
||||
expect(created_trackers.last.target).to eq(llm_model.id.to_s)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -38,7 +38,7 @@ RSpec.describe ProblemCheck::AiCreditHardLimit do
|
||||
problems = described_class.new.call
|
||||
|
||||
expect(problems.size).to eq(1)
|
||||
expect(problems.first.identifier).to eq("ai_credit_hard_limit")
|
||||
expect(problems.first.identifier).to eq(:ai_credit_hard_limit)
|
||||
expect(problems.first.priority).to eq("high")
|
||||
expect(problems.first.target).to eq(llm_model.id)
|
||||
end
|
||||
@@ -55,7 +55,7 @@ RSpec.describe ProblemCheck::AiCreditHardLimit do
|
||||
problems = described_class.new.call
|
||||
|
||||
expect(problems.size).to eq(1)
|
||||
expect(problems.first.identifier).to eq("ai_credit_hard_limit")
|
||||
expect(problems.first.identifier).to eq(:ai_credit_hard_limit)
|
||||
end
|
||||
|
||||
it "resets credits before checking if needed" do
|
||||
|
||||
@@ -38,7 +38,7 @@ RSpec.describe ProblemCheck::AiCreditSoftLimit do
|
||||
problems = described_class.new.call
|
||||
|
||||
expect(problems.size).to eq(1)
|
||||
expect(problems.first.identifier).to eq("ai_credit_soft_limit")
|
||||
expect(problems.first.identifier).to eq(:ai_credit_soft_limit)
|
||||
expect(problems.first.priority).to eq("low")
|
||||
expect(problems.first.target).to eq(llm_model.id)
|
||||
end
|
||||
|
||||
@@ -50,19 +50,14 @@ RSpec.describe ProblemCheck::AiLlmStatus do
|
||||
},
|
||||
)
|
||||
|
||||
expect(described_class.new.call).to contain_exactly(
|
||||
have_attributes(
|
||||
identifier: "ai_llm_status",
|
||||
target: llm_model.id,
|
||||
priority: "high",
|
||||
message: message,
|
||||
details: {
|
||||
model_id: llm_model.id,
|
||||
model_name: llm_model.display_name,
|
||||
url: "/admin/plugins/discourse-ai/ai-llms/#{llm_model.id}/edit",
|
||||
error: JSON.parse(error_response)["message"],
|
||||
},
|
||||
),
|
||||
expect(described_class.new.call.first).to have_attributes(
|
||||
identifier: :ai_llm_status,
|
||||
target: llm_model.id,
|
||||
priority: "high",
|
||||
message: message,
|
||||
details: {
|
||||
error: JSON.parse(error_response)["message"],
|
||||
},
|
||||
)
|
||||
end
|
||||
|
||||
@@ -113,7 +108,7 @@ RSpec.describe ProblemCheck::AiLlmStatus do
|
||||
problems = described_class.new.call
|
||||
expect(problems.length).to eq(1)
|
||||
expect(problems.first).to have_attributes(
|
||||
identifier: "ai_llm_status",
|
||||
identifier: :ai_llm_status,
|
||||
target: llm_model.id,
|
||||
priority: "high",
|
||||
)
|
||||
@@ -125,7 +120,7 @@ RSpec.describe ProblemCheck::AiLlmStatus do
|
||||
problems = described_class.new.call
|
||||
expect(problems.length).to eq(1)
|
||||
expect(problems.first).to have_attributes(
|
||||
identifier: "ai_llm_status",
|
||||
identifier: :ai_llm_status,
|
||||
target: llm_model.id,
|
||||
priority: "high",
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user