From cf9e5c257b506089daea9574f3f0b675849780cb Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 28 Sep 2026 15:41:06 -0400 Subject: [PATCH 1/4] Spam: use Jev as a cheap escalation check for spam without links (#23885) * Use Jev as a cheap escalation check for spam without links Article and comment spam checks only reached Gemini (ArticleCheck/CommentCheck) when the content had an tag, so spam with contact details written as text (Telegram, WhatsApp, phone) never got an AI check. Content without links now goes to a Jev check (TypeSafe's classifier) first, and only escalates to the existing Gemini check when Jev flags it. Jev never flags anything on its own. Without TYPESAFE_API_KEY nothing changes. Claude-Session: https://claude.ai/code/session_01FLx46puMiJkQWtNe7kTRJU * Move the spam escalation check onto Ai::TypeSafe and Ai::FunctionConfig Replaces the PR's standalone Ai::Jev client with the shared Ai::TypeSafe::Client, Questions and Result from #23879, so the escalation check gets the same retries, AiAudit logging and model pinning (TYPESAFE_API_MODEL) as every other Jev function. Escalation is now the :spam_escalation AI function, selectable under Config > AI Models. It is Jev-only: its choices are Off (the default) and Jev, so setting TYPESAFE_API_KEY for other functions no longer turns it on implicitly. The spam handler gates on Ai::FunctionConfig.available? for the article/comment spam checks, so escalated content goes to whichever model those checks are set to, Gemini or Jev. Co-Authored-By: Claude Opus 5.5 * Bound Jev latency in high-priority jobs and make the escalation threshold admin-tunable The spam and moderation jobs (Articles/Comments::HandleSpamWorker, DetectCodeBlockLanguagesWorker) run on high_priority and can make several Jev calls each. With a 20s timeout and 3 retries, a TypeSafe outage could hold a worker for ~87s per call. - Ai::TypeSafe::Client takes timeout: and max_retries:, defaults to the SDKs' 10s timeout, and exposes FAIL_FAST (5s, no retries). Every Jev caller on a high-priority path uses it: ArticleCheck, CommentCheck, SpamEscalationCheck, ContentModerationLabeler, SubforemFinder, ArticleEnhancer and DetectCodeBlockLanguages. Background callers keep retries. - A circuit breaker shared through Rails.cache opens after 5 outage failures (timeouts, connection errors, 429, 5xx) in a minute and skips all Jev calls for 2 minutes. - DetectCodeBlockLanguages aborts when the circuit is open instead of writing "plaintext" for every remaining block. - The spam escalation threshold is a global Settings::AiFunctions value (default 0.3, validated to (0, 1]) editable under Config > AI Models > Tuning. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Ben Halpern Co-authored-by: Claude Opus 5.5 --- .../admin/settings/ai_functions_controller.rb | 14 ++- app/helpers/admin/settings_helper.rb | 8 +- app/models/settings/ai_functions.rb | 19 ++++ app/services/ai/article_check.rb | 2 +- app/services/ai/article_enhancer.rb | 2 +- app/services/ai/comment_check.rb | 2 +- app/services/ai/content_moderation_labeler.rb | 2 +- app/services/ai/function_config.rb | 13 ++- app/services/ai/function_registry.rb | 22 ++++- app/services/ai/spam_escalation_check.rb | 61 +++++++++++++ app/services/ai/subforem_finder.rb | 2 +- app/services/ai/type_safe/client.rb | 65 +++++++++++-- .../articles/detect_code_block_languages.rb | 5 +- app/services/spam/handler.rb | 14 ++- .../settings/forms/_ai_functions.html.erb | 16 ++++ spec/models/settings/ai_functions_spec.rb | 21 +++++ .../admin/settings/ai_functions_spec.rb | 26 ++++++ spec/services/ai/function_config_spec.rb | 15 +++ .../services/ai/spam_escalation_check_spec.rb | 91 +++++++++++++++++++ spec/services/ai/type_safe/client_spec.rb | 76 ++++++++++++++++ .../detect_code_block_languages_spec.rb | 9 ++ spec/services/spam/handler_spec.rb | 84 +++++++++++++++++ 22 files changed, 537 insertions(+), 32 deletions(-) create mode 100644 app/services/ai/spam_escalation_check.rb create mode 100644 spec/services/ai/spam_escalation_check_spec.rb diff --git a/app/controllers/admin/settings/ai_functions_controller.rb b/app/controllers/admin/settings/ai_functions_controller.rb index 81c7eb0ffa37d..1ee3742d32c15 100644 --- a/app/controllers/admin/settings/ai_functions_controller.rb +++ b/app/controllers/admin/settings/ai_functions_controller.rb @@ -17,15 +17,23 @@ def authorization_resource # Unknown functions and options a function does not support are dropped rather than # stored, so a stale form can never select, say, Jev for a text-generation function. def upsert_config(settings) - selections = Ai::FunctionConfig.sanitize(settings[:function_models].to_h) - ::Settings::AiFunctions.set_global_function_models(selections) + ::Settings::AiFunctions.transaction do + if settings.key?(:function_models) + selections = Ai::FunctionConfig.sanitize(settings[:function_models].to_h) + ::Settings::AiFunctions.set_global_function_models(selections) + end + if settings[:spam_escalation_threshold].present? + ::Settings::AiFunctions.set_global_spam_escalation_threshold(settings[:spam_escalation_threshold]) + end + end Result.new([]) rescue ActiveRecord::RecordInvalid => e Result.new([e.message]) end def settings_params - params.require(:settings_ai_functions).permit(function_models: Ai::FunctionRegistry.configurable.map(&:key)) + params.require(:settings_ai_functions) + .permit(:spam_escalation_threshold, function_models: Ai::FunctionRegistry.configurable.map(&:key)) end end end diff --git a/app/helpers/admin/settings_helper.rb b/app/helpers/admin/settings_helper.rb index ada004a992c4f..e13f849240881 100644 --- a/app/helpers/admin/settings_helper.rb +++ b/app/helpers/admin/settings_helper.rb @@ -13,13 +13,15 @@ def billboard_all_countries_for_editing def ai_function_model_options(function, current) options = Ai::FunctionConfig.options_for(function) labeled = options.map do |option| - label = Ai::FunctionConfig.option_label(option) - unless Ai::FunctionConfig.option_available?(option) + label = Ai::FunctionConfig.option_label(option, function) + unless Ai::FunctionConfig.option_available?(option, function) label = "#{label} (requires #{Ai::FunctionConfig.missing_key_for(option)})" end [label, option] end - disabled = options.reject { |option| option == current || Ai::FunctionConfig.option_available?(option) } + disabled = options.reject do |option| + option == current || Ai::FunctionConfig.option_available?(option, function) + end options_for_select(labeled, selected: current, disabled: disabled) end diff --git a/app/models/settings/ai_functions.rb b/app/models/settings/ai_functions.rb index b58bca361beb2..b30b16564b21f 100644 --- a/app/models/settings/ai_functions.rb +++ b/app/models/settings/ai_functions.rb @@ -12,6 +12,10 @@ class AiFunctions < Base self.table_name = :settings_ai_functions setting :function_models, type: :hash, default: {} + # Minimum Jev probability at which content without links is escalated to the spam checks + # (see Ai::SpamEscalationCheck). Lower catches more spam at the cost of more spam checks. + setting :spam_escalation_threshold, type: :float, default: 0.3, + validates: { numericality: { greater_than: 0, less_than_or_equal_to: 1 } } class << self # @return [HashWithIndifferentAccess] function key => option key, global rows only. @@ -25,6 +29,21 @@ def function_model(function_key) global_function_models[function_key.to_s].presence end + # @return [Float] The global escalation threshold, or its default. + def global_spam_escalation_threshold + raw = all_settings(nil)["spam_escalation_threshold"] + raw.present? ? convert_string_to_value_type(:float, raw) : get_default(:spam_escalation_threshold) + end + + def set_global_spam_escalation_threshold(value) + record = find_by(var: "spam_escalation_threshold", subforem_id: nil) || + new(var: "spam_escalation_threshold", subforem_id: nil) + record.value = value + record.save! + clear_cache + value + end + # Replaces the stored selections. Keys and values are validated by the caller # (Ai::FunctionConfig.sanitize). def set_global_function_models(selections) diff --git a/app/services/ai/article_check.rb b/app/services/ai/article_check.rb index eb11188c07123..2ae5da5ef9bfe 100644 --- a/app/services/ai/article_check.rb +++ b/app/services/ai/article_check.rb @@ -58,7 +58,7 @@ def spam_via_jev? def jev_client Ai::TypeSafe::Client.new(model: @selection.model, wrapper: self, affected_user: @article.user, - affected_content: @article) + affected_content: @article, **Ai::TypeSafe::Client::FAIL_FAST) end def jev_state diff --git a/app/services/ai/article_enhancer.rb b/app/services/ai/article_enhancer.rb index ffcf132ae69b3..fe88900567622 100644 --- a/app/services/ai/article_enhancer.rb +++ b/app/services/ai/article_enhancer.rb @@ -126,7 +126,7 @@ def gemini_client(function_key) def jev_client(function_key) Ai::TypeSafe::Client.new(model: selection(function_key).model, wrapper: self, affected_content: @article, - affected_user: @article.user) + affected_user: @article.user, **Ai::TypeSafe::Client::FAIL_FAST) end # --- Jev (TypeSafe System One) --- diff --git a/app/services/ai/comment_check.rb b/app/services/ai/comment_check.rb index eb27b87583944..3a11ef773c6a3 100644 --- a/app/services/ai/comment_check.rb +++ b/app/services/ai/comment_check.rb @@ -50,7 +50,7 @@ def spam? def spam_via_jev? client = Ai::TypeSafe::Client.new(model: @selection.model, wrapper: self, affected_content: @comment, - affected_user: @comment.user) + affected_user: @comment.user, **Ai::TypeSafe::Client::FAIL_FAST) spam_from_jev?(client.evaluate(state: jev_state, questions: jev_questions)) end diff --git a/app/services/ai/content_moderation_labeler.rb b/app/services/ai/content_moderation_labeler.rb index 9443c1dc95e7c..abca0e15b6344 100644 --- a/app/services/ai/content_moderation_labeler.rb +++ b/app/services/ai/content_moderation_labeler.rb @@ -99,7 +99,7 @@ def evaluate # The client already retries transient failures (429/529/5xx) with backoff. def evaluate_via_jev client = Ai::TypeSafe::Client.new(model: @selection.model, wrapper: self, affected_content: @article, - affected_user: @article.user) + affected_user: @article.user, **Ai::TypeSafe::Client::FAIL_FAST) result = client.evaluate(state: jev_state, questions: jev_questions) { label: label_from_jev(result), compellingness_score: compellingness_from_jev(result) } rescue StandardError => e diff --git a/app/services/ai/function_config.rb b/app/services/ai/function_config.rb index 1aabb4773ca52..ed8f4d2129588 100644 --- a/app/services/ai/function_config.rb +++ b/app/services/ai/function_config.rb @@ -4,7 +4,8 @@ module Ai # # Admins choose a model option per function in Settings::AiFunctions. Resolution: # 1. Use the configured option when the function allows it and its API key is present. - # 2. Otherwise use "default": the function's built-in Gemini behavior, unchanged. + # 2. Otherwise use "default": the function's built-in Gemini behavior, unchanged. For + # Jev-only functions (see Ai::FunctionRegistry) "default" means the function is off. # # With nothing configured every function resolves to "default", so behavior is identical # to before this config existed. @@ -73,11 +74,15 @@ def gemini_model_for(function_key, builtin_model = Ai::Base::DEFAULT_MODEL) def options_for(function) function = Ai::FunctionRegistry.fetch(function) unless function.is_a?(Ai::FunctionRegistry::Function) return [] unless function.configurable? + return %w[default jev] if function.jev_only? OPTIONS.values.filter_map { |option| option.key if function.jev? || !option.jev_only } end - def option_available?(option_key) + # "Off" (the default of a Jev-only function) needs no key. + def option_available?(option_key, function = nil) + return true if option_key.to_s == "default" && function&.jev_only? + case OPTIONS[option_key.to_s]&.provider when :gemini then Ai::Base::DEFAULT_KEY.present? when :typesafe then Ai::TypeSafe::Client::DEFAULT_KEY.present? @@ -85,9 +90,9 @@ def option_available?(option_key) end end - def option_label(option_key) + def option_label(option_key, function = nil) case option_key.to_s - when "default" then "Default (built-in Gemini behavior)" + when "default" then function&.jev_only? ? "Off" : "Default (built-in Gemini behavior)" when "gemini_pro" then "Gemini: #{Ai::Base::DEFAULT_MODEL}" when "gemini_lite" then "Gemini: #{Ai::Base::DEFAULT_LITE_MODEL}" when "jev" then "TypeSafe Jev: #{Ai::TypeSafe::Client::DEFAULT_MODEL}" diff --git a/app/services/ai/function_registry.rb b/app/services/ai/function_registry.rb index 044a8607518ac..83f87ef7af83d 100644 --- a/app/services/ai/function_registry.rb +++ b/app/services/ai/function_registry.rb @@ -10,14 +10,21 @@ module Ai # Functions that write text (summaries, copy, chat replies, trend names) stay on # generative models: System One models return typed judgments, not prose. # + # `jev_only: true` marks functions that exist only as a Jev pass and have no Gemini + # behavior. Their "default" option means the function is off. + # # `configurable: false` marks functions whose model cannot be swapped safely at runtime # (e.g. embeddings, where a different model would invalidate every stored vector). class FunctionRegistry - Function = Struct.new(:key, :name, :description, :group, :jev, :configurable, keyword_init: true) do + Function = Struct.new(:key, :name, :description, :group, :jev, :jev_only, :configurable, keyword_init: true) do def jev? jev end + def jev_only? + jev_only + end + def configurable? configurable end @@ -34,10 +41,17 @@ def configurable? # Moderation & spam { key: :article_spam_check, group: :moderation, jev: true, name: "Article spam check", - description: "Decides whether a linked article from a newer account is clearly spam (Ai::ArticleCheck)." }, + description: "Decides whether an article from a newer account is clearly spam, when it has a link or " \ + "the spam escalation check flags it (Ai::ArticleCheck)." }, { key: :comment_spam_check, group: :moderation, jev: true, name: "Comment spam check", - description: "Decides whether a comment containing a link is clearly spam (Ai::CommentCheck)." }, + description: "Decides whether a comment is clearly spam, when it has a link or the spam escalation " \ + "check flags it (Ai::CommentCheck)." }, + { key: :spam_escalation, group: :moderation, jev: true, jev_only: true, + name: "Spam escalation for content without links", + description: "Sends articles and comments without links to the spam checks above when they look like " \ + "spam, such as Telegram or WhatsApp contacts written as text (Ai::SpamEscalationCheck). " \ + "Off keeps link-only escalation." }, { key: :profile_moderation, group: :moderation, jev: true, name: "Profile moderation label", description: "Flags clear and obvious spam or abuse in new profiles (Ai::ProfileModerationLabeler)." }, @@ -115,7 +129,7 @@ def configurable? { key: :image_generation, group: :fixed, jev: false, configurable: false, name: "Image generation", description: "Generates cover and profile images (Ai::ImageGenerator) with a dedicated image model." }, - ].map { |attrs| Function.new(configurable: true, **attrs).freeze }.index_by(&:key).freeze + ].map { |attrs| Function.new(configurable: true, jev_only: false, **attrs).freeze }.index_by(&:key).freeze class UnknownFunctionError < ArgumentError; end diff --git a/app/services/ai/spam_escalation_check.rb b/app/services/ai/spam_escalation_check.rb new file mode 100644 index 0000000000000..e74be7e7007ca --- /dev/null +++ b/app/services/ai/spam_escalation_check.rb @@ -0,0 +1,61 @@ +module Ai + ## + # A cheap Jev pass that decides whether an article or comment without links should still go + # to the full spam check (Ai::ArticleCheck / Ai::CommentCheck), which makes the final call on + # whichever model it is configured for. It only escalates: a "yes" here never flags anything + # on its own. + # + # Runs only when the :spam_escalation function is set to Jev (see Ai::FunctionConfig); + # otherwise content without links is never escalated, as before. + class SpamEscalationCheck + VERSION = "1.1".freeze + FUNCTION_KEY = :spam_escalation + + MAX_TEXT_LENGTH = 3_000 + + QUESTIONS = { + spam: Ai::TypeSafe::Questions.noul( + "Is `content`, published on a developer community, spam?", + yes: "Advertising, SEO or affiliate content, selling goods, services or accounts, scams, or gibberish, " \ + "rather than a good-faith contribution to the community.", + no: "A good-faith post for developers, even if it mentions the author's own project.", + ), + offplatform_contact: Ai::TypeSafe::Questions.noul( + "Does `content` ask readers to contact someone off this platform to buy, order, or get a service?", + yes: "It gives a Telegram, WhatsApp, Discord, phone number, email, or similar contact for readers " \ + "to buy, order, or get a service.", + no: "It makes no such request.", + ) + }.freeze + + # @param text [String] the content to check + # @param content [Article, Comment] the record the text came from (for the audit log) + def initialize(text:, content:) + @text = text.to_s.first(MAX_TEXT_LENGTH) + @content = content + end + + # @return [Boolean] true if the content should go to the full spam check + def escalate? + selection = Ai::FunctionConfig.selection_for(FUNCTION_KEY) + return false unless selection.jev? + + client = Ai::TypeSafe::Client.new(model: selection.model, wrapper: self, affected_content: @content, + affected_user: @content.user, **Ai::TypeSafe::Client::FAIL_FAST) + result = client.evaluate(state: { content: @text }, questions: QUESTIONS) + QUESTIONS.keys.any? { |id| result.noul(id) >= threshold } + rescue StandardError => e + Rails.logger.error("Spam escalation check failed: #{e}") + false + end + + private + + # Admin-tunable (Settings::AiFunctions). The default favors recall, since a false escalation + # only costs one spam check. Tune it from the probabilities logged in AiAudit against which + # escalations the spam check confirms. + def threshold + Settings::AiFunctions.global_spam_escalation_threshold + end + end +end diff --git a/app/services/ai/subforem_finder.rb b/app/services/ai/subforem_finder.rb index 70a1e170f970a..565f89d55de33 100644 --- a/app/services/ai/subforem_finder.rb +++ b/app/services/ai/subforem_finder.rb @@ -90,7 +90,7 @@ def find_best_match_via_jev(available_subforems) candidates = available_subforems.first(Ai::TypeSafe::Questions::MAX_CHOICE_OPTIONS - 1) .index_by { |subforem| "subforem_#{subforem.id}" } client = Ai::TypeSafe::Client.new(model: @selection.model, wrapper: self, affected_content: article, - affected_user: article.user) + affected_user: article.user, **Ai::TypeSafe::Client::FAIL_FAST) result = client.evaluate(state: jev_state, questions: jev_questions(candidates)) pick = result.choice(:best_home) diff --git a/app/services/ai/type_safe/client.rb b/app/services/ai/type_safe/client.rb index 6417e5ff1ac8c..3f0541b617a66 100644 --- a/app/services/ai/type_safe/client.rb +++ b/app/services/ai/type_safe/client.rb @@ -9,6 +9,12 @@ module TypeSafe # callers should batch every question they might need into a single #evaluate call. # # Every call is recorded as an AiAudit row, mirroring Ai::Base. + # + # Outages: callers on latency-sensitive queues (the high-priority spam and moderation jobs) + # pass FAIL_FAST so one call can hold a worker for at most one short attempt. A circuit + # breaker shared across processes (via Rails.cache) also stops calling TypeSafe for + # CIRCUIT_COOLDOWN after CIRCUIT_FAILURE_THRESHOLD outage failures within CIRCUIT_WINDOW, + # raising CircuitOpenError immediately instead. Every caller fails open on errors. class Client include HTTParty base_uri "https://api.typesafe.ai/v1" @@ -18,10 +24,22 @@ class Client # (e.g. "jev-1.13.0") via TYPESAFE_API_MODEL once thresholds are tuned against it. # `presence` so a blank value from a copied .env file does not become the model name. DEFAULT_MODEL = (ENV["TYPESAFE_API_MODEL"].presence || "jev-latest").freeze - TIMEOUT_SECONDS = 20 + # Matches the TypeSafe SDKs' default per-request timeout. + TIMEOUT_SECONDS = 10 MAX_RETRIES = 3 + # For calls inside high-priority jobs: one short attempt and no retries, the same as the + # Gemini calls on those paths. A missed answer only means the caller falls back. + FAIL_FAST = { timeout: 5, max_retries: 0 }.freeze # 429: rate limited, 529: overloaded. Both are documented as retryable with backoff. RETRYABLE_STATUS_CODES = [429, 500, 502, 503, 504, 529].freeze + NETWORK_ERRORS = [Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Errno::ECONNRESET, + Errno::ECONNREFUSED].freeze + + CIRCUIT_FAILURE_THRESHOLD = 5 + CIRCUIT_WINDOW = 1.minute + CIRCUIT_COOLDOWN = 2.minutes + CIRCUIT_FAILURES_KEY = "ai:type_safe:circuit:failures".freeze + CIRCUIT_OPEN_KEY = "ai:type_safe:circuit:open".freeze class Error < StandardError attr_reader :status_code @@ -32,10 +50,33 @@ def initialize(message, status_code: nil) end end + # Raised without calling TypeSafe while the circuit breaker is open. + class CircuitOpenError < Error; end + + class << self + def circuit_open? + Rails.cache.exist?(CIRCUIT_OPEN_KEY) + end + + # Counts an outage-type failure (timeout, connection error, 429 or 5xx) and opens the + # circuit once there are enough of them in the window. + def record_outage_failure + failures = Rails.cache.increment(CIRCUIT_FAILURES_KEY, 1, expires_in: CIRCUIT_WINDOW) + return unless failures.to_i >= CIRCUIT_FAILURE_THRESHOLD + + Rails.cache.write(CIRCUIT_OPEN_KEY, true, expires_in: CIRCUIT_COOLDOWN) + Rails.cache.delete(CIRCUIT_FAILURES_KEY) + Rails.logger.warn("TypeSafe circuit breaker opened for #{CIRCUIT_COOLDOWN.inspect} after " \ + "#{failures} failures") + rescue StandardError => e + Rails.logger.error("TypeSafe circuit breaker failed to record a failure: #{e}") + end + end + attr_reader :model, :last_response def initialize(api_key: DEFAULT_KEY, model: DEFAULT_MODEL, wrapper: nil, affected_user: nil, - affected_content: nil) + affected_content: nil, timeout: TIMEOUT_SECONDS, max_retries: MAX_RETRIES) raise ArgumentError, "TypeSafe API key cannot be nil" if api_key.blank? && !Rails.env.test? @api_key = api_key @@ -43,6 +84,8 @@ def initialize(api_key: DEFAULT_KEY, model: DEFAULT_MODEL, wrapper: nil, affecte @wrapper = wrapper @affected_user = affected_user @affected_content = affected_content + @timeout = timeout + @max_retries = max_retries end ## @@ -55,6 +98,7 @@ def initialize(api_key: DEFAULT_KEY, model: DEFAULT_MODEL, wrapper: nil, affecte # @return [Ai::TypeSafe::Result] def evaluate(state:, questions:) raise ArgumentError, "At least one question is required" if questions.blank? + raise CircuitOpenError, "TypeSafe circuit breaker is open; skipping the call" if self.class.circuit_open? body = { model: @model, state: state, questions: questions.transform_keys(&:to_s) }.to_json attempt = 0 @@ -62,20 +106,21 @@ def evaluate(state:, questions:) begin start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC) @last_response = nil - @last_response = self.class.post("/systemone", body: body, headers: headers, timeout: TIMEOUT_SECONDS) + @last_response = self.class.post("/systemone", body: body, headers: headers, timeout: @timeout) result = handle_response(@last_response) log_audit(body, retry_count: attempt, latency_ms: elapsed_ms(start_time)) result - rescue Error, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET => e + rescue Error, *NETWORK_ERRORS => e log_audit(body, retry_count: attempt, latency_ms: elapsed_ms(start_time), error_message: e.message) - if retryable?(e) && attempt < MAX_RETRIES - attempt += 1 - sleep(backoff_seconds(attempt)) unless Rails.env.test? - retry - end + raise unless retryable?(e) + + self.class.record_outage_failure + raise if attempt >= @max_retries || self.class.circuit_open? - raise + attempt += 1 + sleep(backoff_seconds(attempt)) unless Rails.env.test? + retry end end diff --git a/app/services/articles/detect_code_block_languages.rb b/app/services/articles/detect_code_block_languages.rb index f4c33573a74cd..6c0e1fd92e29b 100644 --- a/app/services/articles/detect_code_block_languages.rb +++ b/app/services/articles/detect_code_block_languages.rb @@ -103,11 +103,14 @@ def ai_client_unconfigured? def detect_languages_via_jev(unlabeled_blocks) client = Ai::TypeSafe::Client.new(model: selection.model, wrapper: self, affected_content: article, - affected_user: article.user) + affected_user: article.user, **Ai::TypeSafe::Client::FAIL_FAST) unlabeled_blocks.map do |code| answer = client.evaluate(state: { code: code.to_s.strip.first(MAX_BLOCK_CHARS) }, questions: { language: language_question }).choice(:language) answer.confidence >= MIN_JEV_CONFIDENCE ? normalize_language(answer.choice) : "plaintext" + rescue Ai::TypeSafe::Client::CircuitOpenError + # TypeSafe is down: abort rather than write "plaintext" for every remaining block. + raise rescue StandardError => e Rails.logger.error("Jev code block language detection failed for article #{article.id}: #{e}") "plaintext" diff --git a/app/services/spam/handler.rb b/app/services/spam/handler.rb index 6f1c82f6126ff..6cac1f4702c4d 100644 --- a/app/services/spam/handler.rb +++ b/app/services/spam/handler.rb @@ -81,8 +81,9 @@ def self.handle_article!(article:, attributes: %i[title body_markdown]) # Check if we should trigger spam detection should_check = Settings::RateLimit.trigger_spam_for?(text: text) || - (article.processed_html.include?(" <% end %> +
+ Tuning +
+ <%= f.label :spam_escalation_threshold, "Spam escalation threshold", class: "crayons-field__label" %> + <%= admin_config_description "When Spam escalation is set to Jev, content without links goes to the spam " \ + "checks once Jev's probability that it is spam, or that it asks readers to " \ + "make contact off-platform to buy something, reaches this value (0-1). Lower " \ + "catches more spam but runs more spam checks. Tune it against the " \ + "Ai::SpamEscalationCheck rows in AI audits." %> + <%= f.number_field :spam_escalation_threshold, + class: "crayons-textfield", + value: Settings::AiFunctions.global_spam_escalation_threshold, + min: 0.01, max: 1, step: 0.01 %> +
+
+
<%= Ai::FunctionRegistry::GROUPS.fetch(:fixed) %> <% Ai::FunctionRegistry.all.reject(&:configurable?).each do |function| %> diff --git a/spec/models/settings/ai_functions_spec.rb b/spec/models/settings/ai_functions_spec.rb index f59bb649b5a08..942ecd0c38502 100644 --- a/spec/models/settings/ai_functions_spec.rb +++ b/spec/models/settings/ai_functions_spec.rb @@ -16,6 +16,27 @@ expect(described_class.function_model(:article_spam_check)).to eq("jev") end + describe "spam escalation threshold" do + it "defaults to 0.3" do + expect(described_class.global_spam_escalation_threshold).to eq(0.3) + end + + it "is stored globally, regardless of the request's subforem" do + RequestStore.store[:subforem_id] = create(:subforem).id + described_class.set_global_spam_escalation_threshold("0.45") + RequestStore.store[:subforem_id] = nil + + expect(described_class.global_spam_escalation_threshold).to eq(0.45) + end + + it "rejects values outside (0, 1]" do + expect { described_class.set_global_spam_escalation_threshold("0") } + .to raise_error(ActiveRecord::RecordInvalid) + expect { described_class.set_global_spam_escalation_threshold("1.5") } + .to raise_error(ActiveRecord::RecordInvalid) + end + end + it "replaces previous selections" do described_class.set_global_function_models("article_spam_check" => "jev") described_class.set_global_function_models("comment_spam_check" => "jev") diff --git a/spec/requests/admin/settings/ai_functions_spec.rb b/spec/requests/admin/settings/ai_functions_spec.rb index 7f0e960cd2072..eb37a6601843e 100644 --- a/spec/requests/admin/settings/ai_functions_spec.rb +++ b/spec/requests/admin/settings/ai_functions_spec.rb @@ -39,6 +39,30 @@ expect(Settings::AiFunctions.global_function_models).to eq({}) end + it "saves the spam escalation threshold alongside the selections" do + sign_in super_admin + + post admin_settings_ai_functions_path, params: { + settings_ai_functions: { spam_escalation_threshold: "0.5", function_models: { spam_escalation: "jev" } } + } + + expect(response).to have_http_status(:ok) + expect(Settings::AiFunctions.global_spam_escalation_threshold).to eq(0.5) + expect(Ai::FunctionConfig.jev?(:spam_escalation)).to be(true) + end + + it "rejects an invalid threshold without saving the selections" do + sign_in super_admin + + post admin_settings_ai_functions_path, params: { + settings_ai_functions: { spam_escalation_threshold: "2", function_models: { spam_escalation: "jev" } } + } + + expect(response).to have_http_status(:unprocessable_entity) + expect(Settings::AiFunctions.global_function_models).to eq({}) + expect(Settings::AiFunctions.global_spam_escalation_threshold).to eq(0.3) + end + it "is only available to super admins" do sign_in create(:user, :admin) @@ -62,6 +86,8 @@ expect(response.body).to include("AI Models") expect(response.body).to include("settings_ai_functions[function_models][content_moderation]") expect(response.body).to include("TypeSafe Jev: #{Ai::TypeSafe::Client::DEFAULT_MODEL}") + expect(response.body).to include("settings_ai_functions[function_models][spam_escalation]") + expect(response.body).to include("settings_ai_functions[spam_escalation_threshold]") expect(response.body).not_to include("settings_ai_functions[function_models][embeddings]") end end diff --git a/spec/services/ai/function_config_spec.rb b/spec/services/ai/function_config_spec.rb index e54767fbbbc1c..56caea86be384 100644 --- a/spec/services/ai/function_config_spec.rb +++ b/spec/services/ai/function_config_spec.rb @@ -77,11 +77,25 @@ expect(described_class.options_for(:article_summary)).to eq(%w[default gemini_pro gemini_lite]) end + it "offers only Off and Jev for Jev-only functions" do + expect(described_class.options_for(:spam_escalation)).to eq(%w[default jev]) + expect(described_class.option_label("default", Ai::FunctionRegistry.fetch(:spam_escalation))).to eq("Off") + end + it "offers nothing for fixed-model functions" do expect(described_class.options_for(:embeddings)).to eq([]) end end + describe ".option_available?" do + it "needs no key to turn a Jev-only function off" do + stub_const("Ai::Base::DEFAULT_KEY", nil) + + expect(described_class.option_available?("default")).to be(false) + expect(described_class.option_available?("default", Ai::FunctionRegistry.fetch(:spam_escalation))).to be(true) + end + end + describe ".sanitize" do it "keeps only known functions with supported options and drops defaults" do sanitized = described_class.sanitize( @@ -90,6 +104,7 @@ "context_note" => "gemini_lite", "comment_spam_check" => "default", "embeddings" => "gemini_pro", + "spam_escalation" => "gemini_lite", "not_a_function" => "jev", ) diff --git a/spec/services/ai/spam_escalation_check_spec.rb b/spec/services/ai/spam_escalation_check_spec.rb new file mode 100644 index 0000000000000..7eac5812821cf --- /dev/null +++ b/spec/services/ai/spam_escalation_check_spec.rb @@ -0,0 +1,91 @@ +require "rails_helper" + +RSpec.describe Ai::SpamEscalationCheck do + subject(:check) { described_class.new(text: "DM me on Telegram @seller for verified accounts", content: article) } + + let(:article) { create(:article) } + + context "when :spam_escalation is set to Jev" do + before { enable_jev_for(:spam_escalation) } + + it "escalates when any question is at or above the default threshold of 0.3" do + stub_jev(offplatform_contact: 0.3) + + expect(check.escalate?).to be(true) + end + + it "uses the threshold set in admin" do + Settings::AiFunctions.set_global_spam_escalation_threshold(0.6) + + stub_jev(spam: 0.5) + expect(check.escalate?).to be(false) + + stub_jev(spam: 0.6) + expect(check.escalate?).to be(true) + end + + it "makes one short attempt so an outage cannot hold a high-priority worker" do + stub_jev + + check.escalate? + + expect(Ai::TypeSafe::Client).to have_received(:new) + .with(hash_including(timeout: 5, max_retries: 0)) + end + + it "does not escalate when every answer is below the threshold" do + stub_jev(spam: 0.1, offplatform_contact: 0.1) + + expect(check.escalate?).to be(false) + end + + it "sends the truncated text as the state" do + requests = stub_jev + described_class.new(text: "a" * 5_000, content: article).escalate? + + expect(requests.first[:state]).to eq(content: "a" * described_class::MAX_TEXT_LENGTH) + expect(requests.first[:questions].keys).to eq(%i[spam offplatform_contact]) + end + + it "does not escalate when the TypeSafe API fails" do + client = instance_double(Ai::TypeSafe::Client) + allow(Ai::TypeSafe::Client).to receive(:new).and_return(client) + allow(client).to receive(:evaluate).and_raise(Ai::TypeSafe::Client::Error.new("boom", status_code: 400)) + + expect(check.escalate?).to be(false) + end + + it "logs the call to AiAudit under this check" do + allow(Ai::TypeSafe::Client).to receive(:post).and_return( + instance_double(HTTParty::Response, success?: true, code: 200, parsed_response: { + "model" => "jev-1.13.0", + "answers" => { "spam" => { "type" => "noul", "noul" => 0.1 }, + "offplatform_contact" => { "type" => "noul", "noul" => 0.1 } }, + "usage" => { "input_tokens" => 42, "output_tokens" => 0 } + }), + ) + + expect { check.escalate? }.to change(AiAudit, :count).by(1) + expect(AiAudit.last).to have_attributes(ai_model: "jev-1.13.0", wrapper_object_class: described_class.name, + wrapper_object_version: described_class::VERSION, + affected_content: article, prompt_token_count: 42) + end + end + + it "does not escalate or call Jev when :spam_escalation is off, even with a TypeSafe key" do + stub_const("Ai::TypeSafe::Client::DEFAULT_KEY", "test-typesafe-key") + allow(Ai::TypeSafe::Client).to receive(:new) + + expect(check.escalate?).to be(false) + expect(Ai::TypeSafe::Client).not_to have_received(:new) + end + + it "does not escalate when Jev is selected but the TypeSafe key is missing" do + enable_jev_for(:spam_escalation) + stub_const("Ai::TypeSafe::Client::DEFAULT_KEY", nil) + allow(Ai::TypeSafe::Client).to receive(:new) + + expect(check.escalate?).to be(false) + expect(Ai::TypeSafe::Client).not_to have_received(:new) + end +end diff --git a/spec/services/ai/type_safe/client_spec.rb b/spec/services/ai/type_safe/client_spec.rb index 3298d3994c713..4ee4d29e1de15 100644 --- a/spec/services/ai/type_safe/client_spec.rb +++ b/spec/services/ai/type_safe/client_spec.rb @@ -103,6 +103,82 @@ def stub_success expect(a_request(:post, endpoint)).to have_been_made.times(described_class::MAX_RETRIES + 1) end + it "does not retry when asked to fail fast" do + stub_request(:post, endpoint).to_return(status: 529, body: "{}", headers: { "Content-Type" => "application/json" }) + fast_client = described_class.new(api_key: "ts-key", **described_class::FAIL_FAST) + + expect { fast_client.evaluate(state: state, questions: questions) }.to raise_error(described_class::Error, /529/) + expect(a_request(:post, endpoint)).to have_been_made.once + end + + it "passes the per-request timeout to HTTParty" do + allow(described_class).to receive(:post).and_raise(Net::ReadTimeout) + + expect do + described_class.new(api_key: "ts-key", timeout: 5, max_retries: 0).evaluate(state: state, questions: questions) + end.to raise_error(Net::ReadTimeout) + expect(described_class).to have_received(:post).with("/systemone", hash_including(timeout: 5)).once + end + + context "with the circuit breaker" do + let(:fast_client) { described_class.new(api_key: "ts-key", **described_class::FAIL_FAST) } + + before { allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) } + + def fail_outage_calls(count) + count.times do + fast_client.evaluate(state: state, questions: questions) + rescue described_class::Error, *described_class::NETWORK_ERRORS + nil + end + end + + it "opens after repeated outage failures and then skips calls without a request" do + stub_request(:post, endpoint) + .to_return(status: 503, body: "{}", headers: { "Content-Type" => "application/json" }) + fail_outage_calls(described_class::CIRCUIT_FAILURE_THRESHOLD) + + expect(described_class).to be_circuit_open + expect { fast_client.evaluate(state: state, questions: questions) } + .to raise_error(described_class::CircuitOpenError) + expect(a_request(:post, endpoint)).to have_been_made.times(described_class::CIRCUIT_FAILURE_THRESHOLD) + end + + it "counts timeouts as outage failures" do + stub_request(:post, endpoint).to_timeout + fail_outage_calls(described_class::CIRCUIT_FAILURE_THRESHOLD) + + expect(described_class).to be_circuit_open + end + + it "stops retrying once the circuit opens mid-call" do + stub_request(:post, endpoint) + .to_return(status: 529, body: "{}", headers: { "Content-Type" => "application/json" }) + fail_outage_calls(described_class::CIRCUIT_FAILURE_THRESHOLD - 1) + + expect { client.evaluate(state: state, questions: questions) }.to raise_error(described_class::Error, /529/) + expect(a_request(:post, endpoint)).to have_been_made.times(described_class::CIRCUIT_FAILURE_THRESHOLD) + end + + it "does not open on validation errors" do + stub_request(:post, endpoint).to_return(status: 422, body: { detail: "bad question" }.to_json, + headers: { "Content-Type" => "application/json" }) + fail_outage_calls(described_class::CIRCUIT_FAILURE_THRESHOLD) + + expect(described_class).not_to be_circuit_open + end + + it "closes again after the cooldown" do + stub_request(:post, endpoint) + .to_return(status: 503, body: "{}", headers: { "Content-Type" => "application/json" }) + fail_outage_calls(described_class::CIRCUIT_FAILURE_THRESHOLD) + + Timecop.travel(described_class::CIRCUIT_COOLDOWN.from_now + 1.second) + + expect(described_class).not_to be_circuit_open + end + end + it "rejects a response without answers" do stub_request(:post, endpoint).to_return(status: 200, body: { model: "jev-1.13.0" }.to_json, headers: { "Content-Type" => "application/json" }) diff --git a/spec/services/articles/detect_code_block_languages_spec.rb b/spec/services/articles/detect_code_block_languages_spec.rb index 164a06080807b..94df5a93f1d10 100644 --- a/spec/services/articles/detect_code_block_languages_spec.rb +++ b/spec/services/articles/detect_code_block_languages_spec.rb @@ -115,6 +115,15 @@ expect(described_class.language_criteria.keys).to match_array(described_class::SUPPORTED_LANGUAGE_TAGS) end + it "leaves the article unchanged when the TypeSafe circuit breaker is open" do + client = instance_double(Ai::TypeSafe::Client) + allow(Ai::TypeSafe::Client).to receive(:new).and_return(client) + allow(client).to receive(:evaluate).and_raise(Ai::TypeSafe::Client::CircuitOpenError, "open") + + expect { expect(service.call).to be(false) }.not_to(change { article.reload.body_markdown }) + expect(client).to have_received(:evaluate).once + end + it "runs without a Gemini key" do stub_const("Ai::Base::DEFAULT_KEY", nil) stub_jev(language: ["ruby", 0.9]) diff --git a/spec/services/spam/handler_spec.rb b/spec/services/spam/handler_spec.rb index a7f6870ec2909..622a3a36568ab 100644 --- a/spec/services/spam/handler_spec.rb +++ b/spec/services/spam/handler_spec.rb @@ -96,6 +96,77 @@ end end + context "when an article without links goes through the escalation check" do + let(:labeler) do + instance_double(Ai::ContentModerationLabeler, + evaluate: { label: "no_moderation_label", compellingness_score: 0.5 }) + end + + before do + allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(false) + stub_const("Ai::Base::DEFAULT_KEY", "present") + allow(article).to receive(:processed_html).and_return("

DM me on Telegram @seller

") + allow(Ai::ArticleCheck).to receive(:new).with(article) + .and_return(instance_double(Ai::ArticleCheck, spam?: true)) + allow(Ai::ContentModerationLabeler).to receive(:new).and_return(labeler) + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?).and_return(false) + end + + it "runs the spam check when the escalation check flags it" do + allow(Ai::SpamEscalationCheck).to receive(:new) + .and_return(instance_double(Ai::SpamEscalationCheck, escalate?: true)) + expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1) + end + + it "skips the spam check when the escalation check doesn't flag it" do + allow(Ai::SpamEscalationCheck).to receive(:new) + .and_return(instance_double(Ai::SpamEscalationCheck, escalate?: false)) + expect(handler).to eq(:not_spam) + expect(Ai::ArticleCheck).not_to have_received(:new) + end + + it "skips the spam check while :spam_escalation is off, even with a TypeSafe key" do + stub_const("Ai::TypeSafe::Client::DEFAULT_KEY", "test-typesafe-key") + allow(Ai::TypeSafe::Client).to receive(:new) + + expect(handler).to eq(:not_spam) + expect(Ai::TypeSafe::Client).not_to have_received(:new) + expect(Ai::ArticleCheck).not_to have_received(:new) + end + + it "escalates to the spam check when Jev flags off-platform contact" do + enable_jev_for(:spam_escalation) + requests = stub_jev(offplatform_contact: 0.9) + + expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1) + expect(requests.first[:state][:content]).to eq(text_to_check) + end + end + + context "when escalation and the article spam check both run on Jev without a Gemini key" do + before do + stub_const("Ai::Base::DEFAULT_KEY", nil) + enable_jev_for(:spam_escalation, :article_spam_check) + allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(false) + allow(article).to receive(:processed_html).and_return("

DM me on Telegram @seller

") + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?).and_return(false) + end + + it "flags the article when both checks agree" do + requests = stub_jev(offplatform_contact: 0.9, malicious: 0.95) + + expect { handler }.to change { Reaction.where(reactable: article, category: "vomit").count }.by(1) + expect(requests.size).to eq(2) + end + + it "does not flag the article when the spam check disagrees" do + requests = stub_jev(offplatform_contact: 0.9, good_faith: 0.9) + + expect(handler).to eq(:not_spam) + expect(requests.size).to eq(2) + end + end + context "when spam is triggered by linked domain net_score check" do let(:spam_domain) { "bad-seo-site.com" } let!(:linked_domain) { LinkedDomain.create!(host: spam_domain, net_score: -2000) } @@ -550,6 +621,19 @@ it_behaves_like "comment multiple spam offender" end end + + context "when a comment without links is flagged by the escalation check" do + before do + allow(Settings::RateLimit).to receive(:trigger_spam_for?).and_return(false) + allow(comment).to receive(:processed_html).and_return("

WhatsApp +1 555 0100 for verified accounts

") + allow(Ai::SpamEscalationCheck).to receive(:new) + .and_return(instance_double(Ai::SpamEscalationCheck, escalate?: true)) + allow(Ai::CommentCheck).to receive(:new).with(comment) + .and_return(instance_double(Ai::CommentCheck, spam?: true)) + end + + it_behaves_like "comment first-time spam offender" + end end # No changes to .handle_user! tests From 0583da341c8177d98db11e5237a342fea4c54598 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 28 Sep 2026 15:46:03 -0400 Subject: [PATCH 2/4] Run the spam domain check in a worker so banish can't be rolled back by it (#23900) * Run the spam domain check in a worker so banish can't be rolled back by it Granting the spam or suspended role ran Spam::DomainDetector inside the rolify after_add callback. Its `email LIKE '%@domain'` scan can hit the statement timeout on uncommon domains; the QueryCanceled then rolled back the role insert and aborted Moderator::BanishUser right after it created the BanishedUser row. Honeybadger ignores QueryCanceled, so it failed silently and re-banishing hit the same wall. Claude-Session: https://claude.ai/code/session_018TsZkCc8o2SwEbux3MQgqz * Skip domain detection for already-blocked domains and cover the worker BlockDomainAndSuspendUsersWorker blocks the domain before suspending each user, so each resulting detector job now short-circuits on the indexed blocked_email_domains lookup instead of re-scanning users by email. Claude-Session: https://claude.ai/code/session_018TsZkCc8o2SwEbux3MQgqz --- app/models/user.rb | 4 +++- app/services/spam/domain_detector.rb | 4 +++- app/workers/spam/domain_detector_worker.rb | 14 +++++++++++ spec/models/user_spam_detection_spec.rb | 12 +++------- spec/services/spam/domain_detector_spec.rb | 8 +++++++ .../spam/domain_detector_worker_spec.rb | 23 +++++++++++++++++++ 6 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 app/workers/spam/domain_detector_worker.rb create mode 100644 spec/workers/spam/domain_detector_worker_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index 3722018f3e450..77ce7b11e5e80 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1228,7 +1228,9 @@ def update_user_roles_cache(role) # Check for spam patterns when user gets spam or suspended role if role.name.in?(%w[spam suspended]) - Spam::DomainDetector.new(self).check_and_block_domain! + # The domain scan can hit the statement timeout, so keep it out of this + # callback: an error here rolls back the role grant (and aborts a banish). + Spam::DomainDetectorWorker.perform_async(id) calculate_score end diff --git a/app/services/spam/domain_detector.rb b/app/services/spam/domain_detector.rb index 103ae9e0c7ec6..f5041515f98e1 100644 --- a/app/services/spam/domain_detector.rb +++ b/app/services/spam/domain_detector.rb @@ -80,8 +80,10 @@ def check_and_block_domain! private + # Already-blocked domains are skipped so the bulk suspension in + # BlockDomainAndSuspendUsersWorker doesn't re-scan the domain per user. def should_skip_domain? - POPULAR_SHARED_DOMAINS.include?(@email_domain) + POPULAR_SHARED_DOMAINS.include?(@email_domain) || BlockedEmailDomain.exists?(domain: @email_domain) end def spam_pattern_detected? diff --git a/app/workers/spam/domain_detector_worker.rb b/app/workers/spam/domain_detector_worker.rb new file mode 100644 index 0000000000000..bf45e817b3d3b --- /dev/null +++ b/app/workers/spam/domain_detector_worker.rb @@ -0,0 +1,14 @@ +module Spam + class DomainDetectorWorker + include Sidekiq::Job + + sidekiq_options queue: :default, retry: 5 + + def perform(user_id) + user = User.find_by(id: user_id) + return unless user + + Spam::DomainDetector.new(user).check_and_block_domain! + end + end +end diff --git a/spec/models/user_spam_detection_spec.rb b/spec/models/user_spam_detection_spec.rb index 1972af2a2d663..e91de59788550 100644 --- a/spec/models/user_spam_detection_spec.rb +++ b/spec/models/user_spam_detection_spec.rb @@ -17,25 +17,19 @@ context "when user gets spam role" do it "triggers spam detection" do - expect_any_instance_of(Spam::DomainDetector).to receive(:check_and_block_domain!) - - user.add_role(:spam) + expect { user.add_role(:spam) }.to change(Spam::DomainDetectorWorker.jobs, :size).by(1) end end context "when user gets suspended role" do it "triggers spam detection" do - expect_any_instance_of(Spam::DomainDetector).to receive(:check_and_block_domain!) - - user.add_role(:suspended) + expect { user.add_role(:suspended) }.to change(Spam::DomainDetectorWorker.jobs, :size).by(1) end end context "when user gets other roles" do it "does not trigger spam detection" do - expect_any_instance_of(Spam::DomainDetector).not_to receive(:check_and_block_domain!) - - user.add_role(:trusted) + expect { user.add_role(:trusted) }.not_to change(Spam::DomainDetectorWorker.jobs, :size) end end diff --git a/spec/services/spam/domain_detector_spec.rb b/spec/services/spam/domain_detector_spec.rb index cc866ac13512e..2ad4c8ae4a6d7 100644 --- a/spec/services/spam/domain_detector_spec.rb +++ b/spec/services/spam/domain_detector_spec.rb @@ -15,6 +15,14 @@ end end + context "when domain is already blocked" do + before { BlockedEmailDomain.create!(domain: "example.com") } + + it "returns false without enqueuing another block job" do + expect { detector.check_and_block_domain! }.not_to change(Spam::BlockDomainAndSuspendUsersWorker.jobs, :size) + end + end + context "when spam pattern is detected" do let!(:spam_user1) { create(:user, email: "user1@example.com") } let!(:spam_user2) { create(:user, email: "user2@example.com") } diff --git a/spec/workers/spam/domain_detector_worker_spec.rb b/spec/workers/spam/domain_detector_worker_spec.rb new file mode 100644 index 0000000000000..1472259f53c4a --- /dev/null +++ b/spec/workers/spam/domain_detector_worker_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +RSpec.describe Spam::DomainDetectorWorker, type: :worker do + describe "#perform" do + it "runs domain detection for the user" do + user = create(:user) + detector = instance_double(Spam::DomainDetector, check_and_block_domain!: false) + allow(Spam::DomainDetector).to receive(:new).with(user).and_return(detector) + + described_class.new.perform(user.id) + + expect(detector).to have_received(:check_and_block_domain!) + end + + it "returns early when the user does not exist" do + allow(Spam::DomainDetector).to receive(:new) + + described_class.new.perform(999_999) + + expect(Spam::DomainDetector).not_to have_received(:new) + end + end +end From 49409eddcfd97068a8ed6cd7fe37fd6f3c79e427 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 28 Sep 2026 16:51:25 -0400 Subject: [PATCH 3/4] Send app metrics to Better Stack alongside Datadog (env-gated) (#23887) ForemStatsClient calls are forwarded to Datadog unchanged and also written to Better Stack as one structured event each, so monitors built on the app's own metrics can be rebuilt before Datadog is turned off. Inert unless BETTERSTACK_METRICS_SOURCE_TOKEN is set in production. Claude-Session: https://claude.ai/code/session_01JGecM5hifj2QYgVRa7qDm7 --- config/initializers/datadog.rb | 12 ++++++++- lib/betterstack/stats_client.rb | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 lib/betterstack/stats_client.rb diff --git a/config/initializers/datadog.rb b/config/initializers/datadog.rb index 4646d3cf9eb6f..442389597aa11 100644 --- a/config/initializers/datadog.rb +++ b/config/initializers/datadog.rb @@ -23,4 +23,14 @@ c.tracing.instrument :redis, service_name: "#{service_name}-redis" end -ForemStatsClient = Datadog::Statsd.new \ No newline at end of file +stats_client = Datadog::Statsd.new +# Also send metrics to Better Stack when BETTERSTACK_METRICS_SOURCE_TOKEN is set. +if Rails.env.production? && ENV["BETTERSTACK_METRICS_SOURCE_TOKEN"].present? + require Rails.root.join("lib/betterstack/stats_client") + stats_client = Betterstack::StatsClient.new( + ENV.fetch("BETTERSTACK_METRICS_SOURCE_TOKEN"), + ingesting_host: ENV["BETTERSTACK_METRICS_INGESTING_HOST"].presence, + forward_to: stats_client, + ) +end +ForemStatsClient = stats_client # rubocop:disable Naming/ConstantName diff --git a/lib/betterstack/stats_client.rb b/lib/betterstack/stats_client.rb new file mode 100644 index 0000000000000..9d3b52208ccc4 --- /dev/null +++ b/lib/betterstack/stats_client.rb @@ -0,0 +1,45 @@ +require "logtail" +require_relative "log_device" + +module Betterstack + # Stands in for ForemStatsClient (a Datadog::Statsd): every call is passed on to `forward_to` + # unchanged, then written to Better Stack as one structured log event. Only the methods the app + # calls are implemented. Only loaded when BETTERSTACK_METRICS_SOURCE_TOKEN is set + # (see config/initializers/datadog.rb). + class StatsClient + def initialize(source_token, ingesting_host: nil, forward_to: nil) + @logger = Logtail::Logger.new(LogDevice.new(source_token, ingesting_host: ingesting_host)) + @forward_to = forward_to + end + + # Same signatures as Datadog::Statsd, so every existing call site keeps working. + # rubocop:disable Style/OptionHash + def increment(stat, opts = {}) + @forward_to&.increment(stat, opts) + write(:count, stat, opts.fetch(:by, 1), opts) + end + + def count(stat, count, opts = {}) + @forward_to&.count(stat, count, opts) + write(:count, stat, count, opts) + end + + def gauge(stat, value, opts = {}) + @forward_to&.gauge(stat, value, opts) + write(:gauge, stat, value, opts) + end + + def event(title, text, opts = {}) + @forward_to&.event(title, text, opts) + write(:event, title, text, opts) + end + # rubocop:enable Style/OptionHash + + private + + # Tags keep Datadog's "key:value" strings. + def write(type, name, value, opts) + @logger.info(message: name, metric: { type: type, name: name, value: value, tags: Array(opts[:tags]) }) + end + end +end From 6d0df2a317ca048e0753a996b2f11cb1231c842d Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 28 Sep 2026 17:09:16 -0400 Subject: [PATCH 4/4] Spam: auto-mark repeat offenders, fix spam check commit race, log failed spam reactions (#23883) * Auto-mark repeat spam authors, fix spam check commit race, log failed spam reactions - Low-trust authors (<4 badges) with 3+ mascot vomits on clear_and_obvious_* posts in the last month get the spam role, without waiting for moderators to confirm each vomit. - Enqueue Articles::HandleSpamWorker after the transaction commits so the worker can find newly created articles and sees the committed published state. - Log when the mascot's spam reaction fails validation instead of dropping it silently. Claude-Session: https://claude.ai/code/session_01FLx46puMiJkQWtNe7kTRJU Entire-Checkpoint: 01M3D7GD03KRTQBATV6Z11S3TE * Count only active auto-flags for repeat offenders, keep the count database-side - Ignore mascot vomits that moderators have invalidated or archived (valid_or_confirmed). - Pass the flagged articles as a subquery instead of loading their IDs into Ruby. - Specs for invalidated flags and for the failed-reaction warning and its quiet case. Claude-Session: https://claude.ai/code/session_01FLx46puMiJkQWtNe7kTRJU * Note why repeat auto-flagged authors are marked as spam, expand spam handler tests Leave an automatic_spam note from the mascot (with the flag count) when a repeat auto-flagged author gets the spam role, skipping it when the author is already spam so queued jobs don't duplicate notes. Add specs for the note, each exclusion rule of the repeat-offender count, and the after-commit enqueue. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Ben Halpern Co-authored-by: Claude Opus 5.5 --- app/models/article.rb | 3 +- app/services/spam/handler.rb | 67 ++++++++++--- config/locales/services/en.yml | 1 + config/locales/services/fr.yml | 1 + config/locales/services/pt.yml | 1 + spec/models/article_spec.rb | 47 +++++++++ spec/services/spam/handler_spec.rb | 153 +++++++++++++++++++++++++++++ 7 files changed, 261 insertions(+), 12 deletions(-) diff --git a/app/models/article.rb b/app/models/article.rb index 3b5f49b41e902..81b36b9e3207e 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -1800,7 +1800,8 @@ def create_conditional_autovomits saved_change_to_published? || published_at > 1.minute.ago - Articles::HandleSpamWorker.perform_async(id) + # Enqueue after commit so the worker sees the saved article (and its published state). + ActiveRecord.after_all_transactions_commit { Articles::HandleSpamWorker.perform_async(id) } end def async_bust diff --git a/app/services/spam/handler.rb b/app/services/spam/handler.rb index 6cac1f4702c4d..88b692dda00c8 100644 --- a/app/services/spam/handler.rb +++ b/app/services/spam/handler.rb @@ -26,6 +26,7 @@ module Handler "forex signals", "loan shark", ].freeze + CLEAR_VIOLATION_LABELS = %w[clear_and_obvious_spam clear_and_obvious_harmful clear_and_obvious_inciting].freeze # @return [TrueClass] if we are going to try to use more rigorous spam handling # @return [FalseClass] if we are using less rigorous spam handling def self.more_rigorous_user_profile_spam_checking? @@ -54,15 +55,9 @@ def self.handle_article!(article:, attributes: %i[title body_markdown]) end # Handle clear and obvious violations immediately - if %w[clear_and_obvious_spam clear_and_obvious_harmful clear_and_obvious_inciting].include?(article.automod_label) + if CLEAR_VIOLATION_LABELS.include?(article.automod_label) issue_spam_reaction_for!(reactable: article) - - if Reaction.user_has_been_given_too_many_spammy_article_reactions?( - user: article.user, - include_user_profile: more_rigorous_user_profile_spam_checking?, - ) - suspend!(user: article.user) - end + escalate_clear_violation_author!(user: article.user) return :spam end @@ -199,11 +194,59 @@ def self.suspend!(user:) # # @param reactable [ActiveRecord::Base] def self.issue_spam_reaction_for!(reactable:) - Reaction.create( + reaction = Reaction.create( user_id: Settings::General.mascot_user_id, reactable: reactable, category: "vomit", ) + return if reaction.persisted? || reaction.errors.of_kind?(:user_id, :taken) + + Rails.logger.warn("Spam reaction not created for #{reactable.class.name} #{reactable.id}: " \ + "#{reaction.errors.full_messages.to_sentence}") + end + + def self.escalate_clear_violation_author!(user:) + if Reaction.user_has_been_given_too_many_spammy_article_reactions?( + user: user, + include_user_profile: more_rigorous_user_profile_spam_checking?, + ) + suspend!(user: user) + elsif repeat_auto_flagged_author?(user: user) + mark_repeat_auto_flagged_author_as_spam!(user: user) + end + end + + # Low-trust authors whose recent posts keep earning clear-violation labels (and the mascot's + # vomit) are treated as spammers without waiting for a moderator to confirm each reaction. + def self.repeat_auto_flagged_author?(user:, threshold: 2) + return false if user.badge_achievements_count >= 4 + + recent_auto_flagged_article_count(user: user) > threshold + end + + def self.recent_auto_flagged_article_count(user:) + flagged_articles = user.articles.published + .where("published_at > ?", 1.month.ago) + .where(automod_label: CLEAR_VIOLATION_LABELS) + Reaction.article_vomits.valid_or_confirmed + .where(user_id: Settings::General.mascot_user_id, reactable_id: flagged_articles.select(:id)) + .count + end + + # Leave a note so moderators can see why the account was marked as spam. Skipped when the + # author is already spam so queued jobs for the same author don't pile up duplicate notes. + def self.mark_repeat_auto_flagged_author_as_spam!(user:) + return if user.spam? + + user.add_role(:spam) + + Note.create( + author_id: Settings::General.mascot_user_id, + noteable: user, + reason: "automatic_spam", + content: I18n.t("services.spam.article_handler.marked_spam_repeat_auto_flags", + count: recent_auto_flagged_article_count(user: user)), + ) end # NEW/private: Helper method to check for extensive domain-based spam. @@ -303,7 +346,7 @@ def self.check_subforem_reassignment(article) # NEW/private: Determine if a profile label is a clear violation. def self.clear_profile_violation_label?(label) - %w[clear_and_obvious_spam clear_and_obvious_harmful clear_and_obvious_inciting].include?(label) + CLEAR_VIOLATION_LABELS.include?(label) end # NEW/private: Skip profile checks for established accounts. @@ -376,6 +419,8 @@ def self.extract_all_domains_from(html, limit: 25) :offtopic_label?, :check_subforem_reassignment, :clear_profile_violation_label?, :eligible_for_profile_spam_check?, :published_articles_over_limit?, :published_comments_over_limit?, - :article_linked_domain_spam?, :extract_all_domains_from + :article_linked_domain_spam?, :extract_all_domains_from, + :escalate_clear_violation_author!, :repeat_auto_flagged_author?, + :recent_auto_flagged_article_count, :mark_repeat_auto_flagged_author_as_spam! end end diff --git a/config/locales/services/en.yml b/config/locales/services/en.yml index 2a69d5b90f8da..2f8ae4973af99 100644 --- a/config/locales/services/en.yml +++ b/config/locales/services/en.yml @@ -116,6 +116,7 @@ en: spam: article_handler: suspended_too_many: User suspended for too many spammy articles, triggered by autovomit. + marked_spam_repeat_auto_flags: User marked as spam after %{count} of their articles from the last month were auto-flagged as clear violations, triggered by autovomit. user_subscriptions: create_from_controller_params: source_not_found: Source not found. diff --git a/config/locales/services/fr.yml b/config/locales/services/fr.yml index 657f984a60733..90b1fe7d12d63 100644 --- a/config/locales/services/fr.yml +++ b/config/locales/services/fr.yml @@ -114,6 +114,7 @@ fr: spam: article_handler: suspended_too_many: Utilisateur suspendu pour trop d'articles spam, déclenché par autovomit. + marked_spam_repeat_auto_flags: Utilisateur marqué comme spam après que %{count} de ses articles du dernier mois ont été signalés automatiquement comme des violations évidentes, déclenché par autovomit. user_subscriptions: create_from_controller_params: source_not_found: Source non trouvée. diff --git a/config/locales/services/pt.yml b/config/locales/services/pt.yml index da62b3c49219f..b4cce536fa90a 100644 --- a/config/locales/services/pt.yml +++ b/config/locales/services/pt.yml @@ -124,3 +124,4 @@ pt: spam: article_handler: suspended_too_many: Muitos artigos suspensos + marked_spam_repeat_auto_flags: Usuário marcado como spam depois que %{count} de seus artigos do último mês foram sinalizados automaticamente como violações claras, acionado por autovomit. diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 8ee18f24b45f6..3796fc622fb4e 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -2249,6 +2249,53 @@ def build_and_validate_article(*args) end end end + + # The worker runs on another connection, so it must not be enqueued before the save commits. + context "when saved inside a transaction" do + it "waits for the outermost transaction to commit before enqueueing" do + new_article = nil + sidekiq_assert_enqueued_jobs(1, only: worker) do + described_class.transaction do + new_article = create(:article, published: true) + expect(worker.jobs.pluck("args")).not_to include([new_article.id]) + end + end + expect(worker.jobs.pluck("args")).to include([new_article.id]) + end + + it "does not enqueue when the transaction is rolled back" do + sidekiq_assert_no_enqueued_jobs(only: worker) do + described_class.transaction do + article.update(title: "rolled back title") + raise ActiveRecord::Rollback + end + end + end + end + + context "when a draft is published" do + let(:mascot_user) { create(:user) } + let(:draft) { create(:article, published: false) } + + before do + allow(Settings::General).to receive(:mascot_user_id).and_return(mascot_user.id) + stub_const("Ai::Base::DEFAULT_KEY", "present") + labeler = instance_double(Ai::ContentModerationLabeler, + evaluate: { label: "clear_and_obvious_spam", compellingness_score: 0.5 }) + allow(Ai::ContentModerationLabeler).to receive(:new).and_return(labeler) + end + + it "lets the worker issue the mascot's vomit on the now-published article" do + sidekiq_assert_enqueued_with(job: worker, args: [draft.id]) do + draft.update!(body_markdown: draft.body_markdown.sub("published: false", "published: true")) + end + + sidekiq_perform_enqueued_jobs(only: worker) + + expect(draft.reload.automod_label).to eq("clear_and_obvious_spam") + expect(Reaction.where(user: mascot_user, reactable: draft, category: "vomit")).to exist + end + end end diff --git a/spec/services/spam/handler_spec.rb b/spec/services/spam/handler_spec.rb index 622a3a36568ab..8cfb023b682f0 100644 --- a/spec/services/spam/handler_spec.rb +++ b/spec/services/spam/handler_spec.rb @@ -310,6 +310,159 @@ expect(handler).to eq(:spam) end end + + # Two earlier flags plus the mascot's vomit on the current article make three, which is enough + # to mark the author. Each "does not count" example breaks exactly one of the earlier flags. + context "with a low-trust author whose earlier posts were already auto-flagged" do + let(:author) { article.user } + let(:earlier_articles) { create_list(:article, 2, user: author) } + let(:earlier_flags) { Reaction.where(user: mascot_user, reactable: earlier_articles) } + let(:auto_spam_notes) { Note.where(noteable: author, reason: "automatic_spam") } + let(:other_user) { create(:user) } + + before do + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?) + .with(user: author, include_user_profile: false).and_return(false) + earlier_articles.each do |earlier| + earlier.update_column(:automod_label, "clear_and_obvious_spam") + create(:reaction, user: mascot_user, reactable: earlier, category: "vomit") + end + end + + it "marks the author as spam without waiting for moderator confirmation" do + handler + expect(author.reload).to be_spam + expect(author).not_to be_suspended + end + + it "leaves a note from the mascot explaining why the author was marked as spam" do + expect { handler }.to change(auto_spam_notes, :count).by(1) + + note = auto_spam_notes.last + expect(note.author_id).to eq(mascot_user.id) + expect(note.content).to eq( + I18n.t("services.spam.article_handler.marked_spam_repeat_auto_flags", count: 3), + ) + end + + it "does not leave an automatic_suspend note" do + handler + expect(Note.where(noteable: author, reason: "automatic_suspend")).to be_empty + end + + it "does not add another note when the author is already marked as spam" do + author.add_role(:spam) + expect { handler }.not_to change(auto_spam_notes, :count) + expect(author.reload).to be_spam + end + + it "leaves a single note when another flagged post is handled afterwards" do + handler + later_article = create(:article, user: author) + described_class.handle_article!(article: later_article) + + expect(later_article.reload.automod_label).to eq("clear_and_obvious_spam") + expect(auto_spam_notes.count).to eq(1) + end + + it "suspends instead when the author already has too many confirmed flags" do + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?) + .with(user: author, include_user_profile: false).and_return(true) + handler + expect(author.reload).to be_suspended + expect(author).not_to be_spam + expect(auto_spam_notes).to be_empty + end + + it "marks authors with 3 badges" do + author.update_column(:badge_achievements_count, 3) + handler + expect(author.reload).to be_spam + end + + it "leaves authors with 4 or more badges alone" do + author.update_column(:badge_achievements_count, 4) + handler + expect(author.reload).not_to be_spam + expect(auto_spam_notes).to be_empty + end + + it "counts auto-flags that moderators have confirmed" do + earlier_flags.update_all(status: "confirmed") + handler + expect(author.reload).to be_spam + end + + it "counts posts labeled harmful or inciting" do + earlier_articles.first.update_column(:automod_label, "clear_and_obvious_harmful") + earlier_articles.second.update_column(:automod_label, "clear_and_obvious_inciting") + handler + expect(author.reload).to be_spam + end + + it "ignores auto-flags that moderators have invalidated" do + earlier_flags.update_all(status: "invalid") + handler + expect(author.reload).not_to be_spam + expect(auto_spam_notes).to be_empty + end + + it "does not mark the author with only two auto-flags" do + earlier_flags.first.destroy + handler + expect(author.reload).not_to be_spam + expect(auto_spam_notes).to be_empty + end + + it "does not count posts published more than a month ago" do + earlier_articles.first.update_column(:published_at, 2.months.ago) + handler + expect(author.reload).not_to be_spam + end + + it "does not count posts that are no longer published" do + earlier_articles.first.update_column(:published, false) + handler + expect(author.reload).not_to be_spam + end + + it "does not count posts whose label is no longer a clear violation" do + earlier_articles.first.update_column(:automod_label, "likely_spam") + handler + expect(author.reload).not_to be_spam + end + + it "does not count vomits from users other than the mascot" do + earlier_flags.first.update_column(:user_id, other_user.id) + handler + expect(author.reload).not_to be_spam + end + + it "does not count auto-flags on other authors' posts" do + earlier_articles.first.update_column(:user_id, other_user.id) + handler + expect(author.reload).not_to be_spam + end + end + + context "when the mascot's reaction isn't created" do + before do + allow(Reaction).to receive(:user_has_been_given_too_many_spammy_article_reactions?).and_return(false) + allow(Rails.logger).to receive(:warn) + end + + it "logs a warning when the reaction fails validation" do + allow(article).to receive(:published).and_return(false) + handler + expect(Rails.logger).to have_received(:warn).with(/Spam reaction not created for Article #{article.id}/) + end + + it "stays quiet when the mascot has already reacted" do + create(:reaction, user: mascot_user, reactable: article, category: "vomit") + handler + expect(Rails.logger).not_to have_received(:warn).with(/Spam reaction not created/) + end + end end context "when content moderation labeler identifies clear and obvious harmful content" do