From c9fbfd9f71fc32f2e9171d467e70adfc3949ebb1 Mon Sep 17 00:00:00 2001 From: Andrew B <1974197+andrewb-nz@users.noreply.github.com> Date: Sat, 15 Aug 2026 16:12:22 +1200 Subject: [PATCH] fix(chat): make the assistant response timeout configurable (#2910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(chat): make the assistant response timeout configurable (#2893) Self-hosted users running a local model report the chat failing with "assistant not available" after 90 seconds even though the model generates a reply and tokens are billed. Three timeouts are involved and only one was configurable: - OPENAI_REQUEST_TIMEOUT (60s) — already settable, not the blocker. - The browser watchdog in chat_controller.js (90s) — hardcoded, and this is what actually fires. - Chat::UNDELIVERED_RESPONSE_TIMEOUT (60s) — a constant, so raising the client value alone would not have helped. The watchdog cannot be avoided by streaming here: custom OpenAI-compatible providers route through generic_chat_response, which forces synchronous calls, so nothing renders until the whole generation finishes. Time-to-last-token has to beat the deadline. Adds AI_RESPONSE_TIMEOUT (ENV > Setting > 90s default, floored at 30s), exposed on the Self-Hosting settings page and passed to the Stimulus controller at all three mount points — show, index and the sidebar in the application layout, each of which declares data-controller="chat" independently. The server floor is derived from the same value but kept 10s below it. report_timeout answers 200 whether or not it acted and the client only retries on a non-ok response, so a floor at or above the client value would let clock skew strand a pending bubble permanently. Also guards AssistantMessage#append_text!. The watchdog runs in the web process while the job holds its own copy of the message, so a job finishing after the bubble was destroyed or demoted would silently resurrect it alongside the error the user was already shown. * fix(chat): let the watchdog retry when report_timeout declines `report_timeout` answered 200 whether or not `handle_undelivered_response!` acted. The Stimulus watchdog only stops retrying a URL once it sees a 2xx, so a declined report was treated as final. That stranded the bubble whenever the client's clock ran more than SERVER_TIMEOUT_GRACE ahead of the server's: the watchdog posts at its own timeout, the server sees a message younger than its floor and no-ops, and nothing ever retries. The bubble spins forever with no error and no Retry. Answering 409 instead lets the next 5s tick try again, so any amount of skew costs retries rather than a stuck chat. The grace window stays as an optimisation to keep those retries rare, not as the correctness mechanism. * docs(chat): correct the AI_RESPONSE_TIMEOUT ordering guidance The docs, locale string and examples all said to keep OPENAI_REQUEST_TIMEOUT at or above AI_RESPONSE_TIMEOUT. That is backwards. The two limits span different things. OPENAI_REQUEST_TIMEOUT bounds each HTTP call to the model on its own; AI_RESPONSE_TIMEOUT covers the whole turn and its clock starts when the message is queued, so it also absorbs Sidekiq queue time and, for a tool-using turn, two model calls plus the tool run between them. Keeping the chat timeout the larger of the two means a slow model surfaces the specific HTTP timeout error rather than a generic "no response", and the job stops instead of running on after the chat has given up. The shipped 60/90 defaults already had this ordering; only the guidance was wrong. compose.example.ai.yml gets 300/660 so the Ollama example can actually complete a tool-using turn. * fix(chat): claim the pending bubble atomically before appending append_text! read the row's status and then saved, leaving a window in which the watchdog could demote the row to `failed` between the two. The late content would then land on a bubble the user had already been told failed, flipping it back to `complete`. Replaces the read with a conditional UPDATE that only succeeds while the row is still pending, so the check and the state change cannot be separated. Uses a conditional UPDATE rather than with_lock because append_text! is called once per chunk on the streaming path; a row lock and transaction per chunk would be far more expensive. The claim only runs on the first append, since later ones are no longer pending. * test(chat): isolate AI_RESPONSE_TIMEOUT from the environment Chat.response_timeout reads ENV ahead of Setting, so stubbing only the Setting left the assertions at the mercy of the environment they run in. With AI_RESPONSE_TIMEOUT=45 exported, five of these tests failed — the default, floor and grace assertions were all silently measuring the env value. Adds a with_setting_timeout helper that stubs the Setting and clears the variable together, and switches the controller tests to stub Chat.undelivered_response_timeout directly, since what they care about is the resolved floor rather than how it was configured. Both files now pass with or without AI_RESPONSE_TIMEOUT set. * docs(chat): size AI_RESPONSE_TIMEOUT for chained tool calls The guidance assumed a tool-using turn costs two model calls. #2767 landed after this branch was opened and made tool calls iterative: `Assistant::Responder` now loops until `iteration > max_tool_call_iterations`, so a turn runs to 1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS calls — six by default — with tool execution in between. At the default 60s per-call timeout that is up to 360s of model time against a 90s watchdog. Streaming does not rescue this either. `emit(:output_text)` only fires for a response that carries text, and tool-call-only rounds carry none, so the bubble stays on "Thinking…" through every round regardless of provider. Documents ASSISTANT_MAX_TOOL_CALL_ITERATIONS as the cheaper lever: dropping it to 2 halves the worst case instead of demanding a half-hour timeout, at the cost of failing long tool chains earlier with a clear limit error. compose.example.ai.yml now shows that combination rather than a timeout sized for six calls it never had. * docs(chat): state the whole-turn timeout as a sum, not a maximum The guidance said to keep AI_RESPONSE_TIMEOUT "above" or "the larger of" OPENAI_REQUEST_TIMEOUT. That understates it: the watchdog covers the entire turn, so the bound is (1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS) * OPENAI_REQUEST_TIMEOUT + tool execution + queue wait Merely exceeding the per-call limit can still leave the chat reporting failure while the worker keeps going. One phrasing was outright wrong: "keep AI_RESPONSE_TIMEOUT the largest of the three" compared a duration against ASSISTANT_MAX_TOOL_CALL_ITERATIONS, which is a count, not seconds. Resizes the examples against the formula — compose.example.ai.yml 1000 -> 1200 and the Ollama doc example 600 -> 720, both now showing the arithmetic — and states plainly that the 90s default is sized for typical cloud latency rather than the worst-case bound, with the formula being what matters once per-call latency approaches the timeout. * docs(chat): list the AI settings fields and tag the formula fence The Settings UI walkthrough listed three of the eight fields on the AI Provider form. JSON Mode, the three Token Budget fields and the new Chat Response Timeout were all missing, so the timeout was only discoverable from the troubleshooting section. Rewrites the list to follow the form's own grouping and uses the labels the form actually renders. Also tags the whole-turn formula fence as `text` (markdownlint MD040). * fix(compose): forward ASSISTANT_MAX_TOOL_CALL_ITERATIONS in standard compose This file enumerates container environment explicitly — there is no env_file — so a variable absent from the x-rails-env anchor never reaches web or worker. The tool-call cap was only named in a comment here, while the docs added in 3360dbf5 tell operators to lower it to keep a turn inside AI_RESPONSE_TIMEOUT. Following that advice on this compose file silently changed nothing: the app kept the default of 5 while the timeout was sized for 3 calls, which lands back on the "no response" error this branch exists to fix. Left with an empty default so the app's own default governs, matching OPENAI_MODEL and LLM_CONTEXT_WINDOW above. compose.example.ai.yml already forwarded it. --- .env.example | 23 +++++++ .env.local.example | 2 + app/controllers/messages_controller.rb | 12 +++- .../settings/hostings_controller.rb | 11 ++-- app/javascript/controllers/chat_controller.js | 4 +- app/models/assistant_message.rb | 31 +++++++++- app/models/chat.rb | 47 +++++++++++++-- app/models/setting.rb | 7 +++ app/views/chats/index.html.erb | 2 +- app/views/chats/show.html.erb | 2 +- app/views/layouts/application.html.erb | 2 +- .../hostings/_openai_settings.html.erb | 14 +++++ compose.example.ai.yml | 18 ++++++ compose.example.yml | 9 +++ config/locales/views/settings/hostings/en.yml | 4 ++ docs/hosting/ai.md | 60 +++++++++++++++++-- test/controllers/messages_controller_test.rb | 33 ++++++++++ test/models/assistant_message_test.rb | 43 +++++++++++++ test/models/chat_test.rb | 58 ++++++++++++++++++ 19 files changed, 361 insertions(+), 21 deletions(-) diff --git a/.env.example b/.env.example index b3cd1cb2c..41f98ffc5 100644 --- a/.env.example +++ b/.env.example @@ -35,6 +35,29 @@ OPENAI_URI_BASE= # LLM_SYSTEM_PROMPT_RESERVE=256 # LLM_MAX_ITEMS_PER_CALL=25 +# Optional: how long the chat waits for an assistant response before showing a +# "no response" error. Raise for slow local models — OpenAI-compatible providers +# are not streamed, so nothing renders until the whole reply is generated. +# +# This clock starts when the message is queued and covers the whole turn, so it +# is a SUM, not a maximum. The worst case is: +# +# (1 + ASSISTANT_MAX_TOOL_CALL_ITERATIONS) * OPENAI_REQUEST_TIMEOUT +# + tool execution + queue wait +# +# At the defaults that bound is 6 * 60 = 360s plus overhead. The 90s default is +# sized for typical latency rather than that bound — cloud models answer in +# seconds, so a turn rarely approaches it. Size against the formula once your +# per-call latency is genuinely near OPENAI_REQUEST_TIMEOUT, which is the case +# for local models. Set too low, you get a generic "no response" instead of the +# specific timeout error, and the job keeps burning tokens after the chat gave up. +# Minimum 30; also settable on the Self-Hosting settings page. +# AI_RESPONSE_TIMEOUT=90 +# +# Lowering the tool-call cap is often the better lever on slow hardware: it cuts +# the first term of that sum instead of requiring a very long timeout. +# ASSISTANT_MAX_TOOL_CALL_ITERATIONS=5 + # Optional: OpenAI-compatible capability flags # OPENAI_REQUEST_TIMEOUT=60 # HTTP timeout in seconds; raise for slow local models # OPENAI_SUPPORTS_PDF_PROCESSING=true # Set to false for endpoints without vision support diff --git a/.env.local.example b/.env.local.example index 383fb71ac..8e74d6672 100644 --- a/.env.local.example +++ b/.env.local.example @@ -46,6 +46,8 @@ OPENAI_MODEL = # OpenAI-compatible capability flags (custom/self-hosted providers) # OPENAI_REQUEST_TIMEOUT = 60 # HTTP timeout in seconds; raise for slow local models +# AI_RESPONSE_TIMEOUT = 90 # Whole-turn budget: (1 + iterations) * OPENAI_REQUEST_TIMEOUT + tool time + queue wait +# ASSISTANT_MAX_TOOL_CALL_ITERATIONS = 5 # Chained tool calls per turn; a turn costs up to (1 + this) model calls # OPENAI_SUPPORTS_PDF_PROCESSING = true # Set to false for endpoints without vision support # OPENAI_SUPPORTS_RESPONSES_ENDPOINT = # true to force Responses API on custom providers # LLM_JSON_MODE = # auto | strict | json_object | none diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 252d6645c..0e414a5cb 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -24,8 +24,16 @@ class MessagesController < ApplicationController # current user's chat. def report_timeout message = @chat.messages.find(params[:id]) - @chat.handle_undelivered_response!(message) - head :ok + + if @chat.handle_undelivered_response!(message) + head :ok + else + # Declined — the message has not waited past the server's own floor yet, + # which usually means the client's clock runs ahead of ours and it reported + # early. This must not be a 2xx: the watchdog only stops retrying a URL once + # it sees one, so answering OK here would strand the bubble spinning forever. + head :conflict + end rescue ActiveRecord::RecordNotFound head :not_found end diff --git a/app/controllers/settings/hostings_controller.rb b/app/controllers/settings/hostings_controller.rb index f988aca36..a32f9a17e 100644 --- a/app/controllers/settings/hostings_controller.rb +++ b/app/controllers/settings/hostings_controller.rb @@ -1,13 +1,14 @@ class Settings::HostingsController < ApplicationController layout "settings" - # Minimum accepted value for each configurable LLM budget field. Mirrors the + # Minimum accepted value for each configurable numeric LLM field. Mirrors the # `min:` attribute on the form inputs in `_openai_settings.html.erb` so the # controller rejects what the browser-side validator would reject. - LLM_BUDGET_MINIMUMS = { + LLM_NUMERIC_MINIMUMS = { llm_context_window: 256, llm_max_response_tokens: 64, - llm_max_items_per_call: 1 + llm_max_items_per_call: 1, + ai_response_timeout: Chat::MIN_RESPONSE_TIMEOUT.to_i }.freeze guard_feature unless: -> { self_hosted? } @@ -211,7 +212,7 @@ class Settings::HostingsController < ApplicationController end end - LLM_BUDGET_MINIMUMS.each do |key, minimum| + LLM_NUMERIC_MINIMUMS.each do |key, minimum| next unless hosting_params.key?(key) raw = hosting_params[key].to_s.strip if raw.blank? @@ -271,7 +272,7 @@ class Settings::HostingsController < ApplicationController private def hosting_params return ActionController::Parameters.new unless params.key?(:setting) - params.require(:setting).permit(:onboarding_state, :require_email_confirmation, :invite_only_default_family_id, :brand_fetch_client_id, :brand_fetch_high_res_logos, :twelve_data_api_key, :tiingo_api_key, :eodhd_api_key, :alpha_vantage_api_key, :tinkoff_invest_api_key, :rentcast_api_key, :realie_api_key, :openai_access_token, :openai_uri_base, :openai_model, :openai_json_mode, :anthropic_access_token, :anthropic_base_url, :anthropic_model, :llm_provider, :llm_context_window, :llm_max_response_tokens, :llm_max_items_per_call, :exchange_rate_provider, :securities_provider, :syncs_include_pending, :auto_sync_enabled, :auto_sync_time, :external_assistant_url, :external_assistant_token, :external_assistant_agent_id, securities_providers: []) + params.require(:setting).permit(:onboarding_state, :require_email_confirmation, :invite_only_default_family_id, :brand_fetch_client_id, :brand_fetch_high_res_logos, :twelve_data_api_key, :tiingo_api_key, :eodhd_api_key, :alpha_vantage_api_key, :tinkoff_invest_api_key, :rentcast_api_key, :realie_api_key, :openai_access_token, :openai_uri_base, :openai_model, :openai_json_mode, :anthropic_access_token, :anthropic_base_url, :anthropic_model, :llm_provider, :llm_context_window, :llm_max_response_tokens, :llm_max_items_per_call, :ai_response_timeout, :exchange_rate_provider, :securities_provider, :syncs_include_pending, :auto_sync_enabled, :auto_sync_time, :external_assistant_url, :external_assistant_token, :external_assistant_agent_id, securities_providers: []) end def update_assistant_type diff --git a/app/javascript/controllers/chat_controller.js b/app/javascript/controllers/chat_controller.js index dfb750238..ca61305af 100644 --- a/app/javascript/controllers/chat_controller.js +++ b/app/javascript/controllers/chat_controller.js @@ -5,7 +5,9 @@ export default class extends Controller { static values = { // How long a pending "Thinking…" bubble may wait before we assume the // background worker never delivered a response. Generous so slow models or - // tool calls don't trip it. + // tool calls don't trip it. Set from the server (`Chat.response_timeout_ms`) + // so self-hosters running local models can raise it; this default only + // applies if the value is missing from the markup. responseTimeout: { type: Number, default: 90000 }, // How often to re-check pending bubbles. pollInterval: { type: Number, default: 5000 }, diff --git a/app/models/assistant_message.rb b/app/models/assistant_message.rb index a40304d2c..de172ede3 100644 --- a/app/models/assistant_message.rb +++ b/app/models/assistant_message.rb @@ -5,9 +5,38 @@ class AssistantMessage < Message "assistant" end + # Appends streamed (or, for non-streaming providers, whole) response text. + # + # Returns false without saving if the bubble is no longer awaiting a response. + # The watchdog runs in the *web* process and may have destroyed or failed this + # message while the job was still waiting on a slow model; the job holds its + # own in-memory copy and would otherwise silently resurrect a bubble the user + # has already been told failed. Re-checked once, on the first append — later + # appends stay in-memory cheap. def append_text!(text) + return false if destroyed? || frozen? + return false if pending? && !claim! + self.content += text - self.status = :complete if pending? save! end + + private + + # Flips pending -> complete with a conditional UPDATE, so the check and the + # state change cannot be separated by the watchdog's write. Returns false if + # the row is already gone or `failed`, meaning this turn lost the race and + # must not write. A plain read-then-save would leave a window in which the + # watchdog demotes the row between the two, and the late content would land + # on a bubble the user was already told had failed. + # + # Only reached on the first append (later ones are no longer `pending`), so + # a streaming response pays for this once, not once per chunk. + def claim! + return true unless persisted? + + claimed = self.class.where(id: id, status: :pending).update_all(status: "complete").positive? + self.status = :complete if claimed + claimed + end end diff --git a/app/models/chat.rb b/app/models/chat.rb index 6b76f4c95..4a4a1379f 100644 --- a/app/models/chat.rb +++ b/app/models/chat.rb @@ -132,10 +132,47 @@ class Chat < ApplicationRecord assistant.respond_to(message, assistant_message: assistant_message) end - # Minimum age before the server will treat a still-pending response as - # undelivered. The browser watchdog waits longer (default 90s) before it even - # asks, but the client clock is untrusted, so the server enforces its own floor. - UNDELIVERED_RESPONSE_TIMEOUT = 60.seconds + # How long a pending "Thinking…" bubble may wait before the browser watchdog + # reports it as undelivered. Configurable because a local model on slow + # hardware can legitimately take minutes to produce its first token, and the + # custom OpenAI-compatible provider path is non-streaming — so nothing renders + # until the whole generation finishes. + DEFAULT_RESPONSE_TIMEOUT = 90.seconds + + # Floor on the configured value. Below this the watchdog would fire during + # normal cloud-model latency and kill healthy responses. + MIN_RESPONSE_TIMEOUT = 30.seconds + + # How far *below* the client timeout the server's own floor sits, so a client + # whose clock runs modestly ahead is not refused on its first report. This is + # only an optimisation to keep retries rare: correctness for arbitrary skew + # comes from `MessagesController#report_timeout` answering non-OK when it + # declines, which leaves the watchdog free to try again on its next tick. + SERVER_TIMEOUT_GRACE = 10.seconds + + class << self + # Client-side watchdog timeout. Precedence: ENV > Setting > default, with + # non-positive values treated as unset (a 0-second timeout is never meant). + def response_timeout + configured = ENV["AI_RESPONSE_TIMEOUT"].to_s.strip.to_i + configured = Setting.ai_response_timeout.to_i unless configured.positive? + return DEFAULT_RESPONSE_TIMEOUT unless configured.positive? + + [ configured.seconds, MIN_RESPONSE_TIMEOUT ].max + end + + # Same value in milliseconds, for the Stimulus `responseTimeout` value. + def response_timeout_ms + response_timeout.to_i * 1000 + end + + # Minimum age before the server will treat a still-pending response as + # undelivered. The client clock is untrusted, so the server enforces its own + # floor — kept just under the client's so a genuine report is never refused. + def undelivered_response_timeout + response_timeout - SERVER_TIMEOUT_GRACE + end + end # Handles the case where an assistant response was never delivered — the # background worker never ran `AssistantResponseJob` (or it died before it @@ -152,7 +189,7 @@ class Chat < ApplicationRecord resolved = assistant_message.with_lock do next false unless assistant_message.pending? - next false if assistant_message.created_at > UNDELIVERED_RESPONSE_TIMEOUT.ago + next false if assistant_message.created_at > self.class.undelivered_response_timeout.ago if assistant_message.content.blank? assistant_message.destroy! diff --git a/app/models/setting.rb b/app/models/setting.rb index dde71b960..deed1d5a6 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -22,6 +22,13 @@ class Setting < RailsSettings::Base field :llm_context_window, type: :integer, default: ENV["LLM_CONTEXT_WINDOW"]&.to_i field :llm_max_response_tokens, type: :integer, default: ENV["LLM_MAX_RESPONSE_TOKENS"]&.to_i field :llm_max_items_per_call, type: :integer, default: ENV["LLM_MAX_ITEMS_PER_CALL"]&.to_i + + # How long the chat UI waits for an assistant response before treating it as + # undelivered. Self-hosted users running local models on slow hardware need + # this well above the 90s default — a local model that takes minutes to + # generate would otherwise always trip the watchdog. Read via + # `Chat.response_timeout`, which applies ENV > Setting > default precedence. + field :ai_response_timeout, type: :integer, default: ENV["AI_RESPONSE_TIMEOUT"]&.to_i field :external_assistant_url, type: :string field :external_assistant_token, type: :string field :external_assistant_agent_id, type: :string diff --git a/app/views/chats/index.html.erb b/app/views/chats/index.html.erb index 8afa67c57..139e616cc 100644 --- a/app/views/chats/index.html.erb +++ b/app/views/chats/index.html.erb @@ -1,4 +1,4 @@ -
<%= t(".max_items_per_call_help") %>
<%= t(".ai_response_timeout_description") %>
+ + <%= form.number_field :ai_response_timeout, + label: t(".ai_response_timeout_label"), + placeholder: Chat::DEFAULT_RESPONSE_TIMEOUT.to_i.to_s, + value: Setting.ai_response_timeout, + min: Chat::MIN_RESPONSE_TIMEOUT.to_i, + disabled: ENV["AI_RESPONSE_TIMEOUT"].present?, + data: { "auto-submit-form-target": "auto" } %> +<%= t(".ai_response_timeout_help") %>
+