From c6906bffae449b624d634c1b10ab2c70f900085d Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:29:45 +0100 Subject: [PATCH 1/6] feat(arr): make the OpenAI endpoint a live setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base URL and model move into a cell the backend re-reads per request, so an operator can repoint it without a restart. Migration 0028 adds the two columns; DESIGN.md §15 calls both database rows. Construction never depends on the API key — llama.cpp serves without one. --- .../migrations/0028_openai_endpoint.sql | 17 ++ crates/arr-subs/src/lib.rs | 8 + crates/arr-subs/src/openai.rs | 97 +++++++--- crates/arr-subs/src/openai_endpoint.rs | 168 ++++++++++++++++++ crates/arr-subs/tests/openai.rs | 55 +++++- 5 files changed, 318 insertions(+), 27 deletions(-) create mode 100644 crates/arr-db/migrations/0028_openai_endpoint.sql create mode 100644 crates/arr-subs/src/openai_endpoint.rs diff --git a/crates/arr-db/migrations/0028_openai_endpoint.sql b/crates/arr-db/migrations/0028_openai_endpoint.sql new file mode 100644 index 0000000..f8e0a07 --- /dev/null +++ b/crates/arr-db/migrations/0028_openai_endpoint.sql @@ -0,0 +1,17 @@ +-- #220. Where the OpenAI-compatible translation backend points, and which +-- model it names (DESIGN.md §15). That backend is not "OpenAI" — it is +-- anything speaking that request shape, `llama.cpp` and a local gateway +-- included — so which endpoint and which model are in use is something the +-- operator tries and changes, not a property of the deployment fixed at +-- start-up. Both replace bootstrap keys #216 added as an explicit stopgap. +-- +-- Only the API key stays in the environment (§10): it is a secret, and an +-- endpoint that needs no key at all is a valid configuration. + +ALTER TABLE subtitle_settings + -- NULL means the backend's own default, `https://api.openai.com/v1/`. + ADD COLUMN openai_base_url TEXT; + +ALTER TABLE subtitle_settings + -- NULL means the backend's own default model. + ADD COLUMN openai_model TEXT; diff --git a/crates/arr-subs/src/lib.rs b/crates/arr-subs/src/lib.rs index c17fee1..1eacaf3 100644 --- a/crates/arr-subs/src/lib.rs +++ b/crates/arr-subs/src/lib.rs @@ -33,6 +33,10 @@ pub mod google; pub mod model; #[cfg(feature = "translate-openai")] pub mod openai; +// Not gated: the settings surface must hold and validate this backend's +// endpoint whether or not the build compiled the backend in — the +// `subtitle_settings` columns exist either way (#220). +pub mod openai_endpoint; pub mod opensubtitles; pub mod podnapisi; pub mod srt; @@ -54,6 +58,10 @@ pub use model::{ }; #[cfg(feature = "translate-openai")] pub use openai::{OpenAi, OpenAiConfig}; +pub use openai_endpoint::{ + OpenAiEndpoint, DEFAULT_BASE_URL as OPENAI_DEFAULT_BASE_URL, + DEFAULT_MODEL as OPENAI_DEFAULT_MODEL, +}; pub use opensubtitles::{moviehash, OpenSubtitles, OpenSubtitlesConfig}; pub use podnapisi::{Podnapisi, PodnapisiBuilder, DEFAULT_BASE_URL as PODNAPISI_DEFAULT_BASE_URL}; pub use srt::Cue; diff --git a/crates/arr-subs/src/openai.rs b/crates/arr-subs/src/openai.rs index 5453997..eb28f7d 100644 --- a/crates/arr-subs/src/openai.rs +++ b/crates/arr-subs/src/openai.rs @@ -3,10 +3,12 @@ //! One HTTP shape — the `/chat/completions` endpoint `OpenAI` defined — reaches //! the largest number of options: `OpenAI` itself, `OpenRouter`, Groq, a //! self-hosted gateway, or a local `llama.cpp` server. The model and base URL -//! are constructor arguments rather than fields on [`OpenAiConfig`]: which -//! engine runs, and where, is a database setting (DESIGN.md §15, issue #198). -//! Only the credential is bootstrap config or environment, per §10 — and even -//! that is optional, since a self-hosted gateway may not ask for one. +//! live in an [`OpenAiEndpoint`] cell re-read per request rather than on +//! [`OpenAiConfig`]: both are database rows the operator edits from +//! `/settings`, and an edit reaches this running backend without a restart +//! (DESIGN.md §15, issues #198 and #220). Only the credential is bootstrap +//! config or environment, per §10 — and even that is optional, since a +//! self-hosted gateway or a local `llama.cpp` may not ask for one. //! //! The batch travels as a JSON array of `{number, text}` objects, and the //! reply is asked for in the same shape: JSON round-trips a cue's embedded @@ -20,17 +22,17 @@ use std::fmt; use std::time::Duration; -use reqwest::{Client, StatusCode, Url}; +use reqwest::{Client, StatusCode}; use serde::{Deserialize, Serialize}; use arr_core::Language; +use crate::openai_endpoint::OpenAiEndpoint; use crate::translate::{ strip_code_fence, system_prompt, Backend, BackendId, Batch, Error, ProbeFuture, Result, TranslateFuture, TranslatedCue, }; -const DEFAULT_BASE_URL: &str = "https://api.openai.com/v1/"; const BACKEND_NAME: &str = "openai"; const USER_AGENT: &str = "arr v0.1.0"; const MAX_ERROR_BODY: usize = 512; @@ -58,35 +60,40 @@ impl fmt::Debug for OpenAiConfig { /// The OpenAI-compatible backend behind the [`Backend`] trait. pub struct OpenAi { http: Client, - base_url: Url, + endpoint: OpenAiEndpoint, config: OpenAiConfig, - model: String, id: BackendId, } // Hand-written: the config must never reach a log line. impl fmt::Debug for OpenAi { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let endpoint = self.endpoint.read(); f.debug_struct("OpenAi") - .field("base_url", &self.base_url.as_str()) - .field("model", &self.model) + .field("base_url", &endpoint.base_url.as_str()) + .field("model", &endpoint.model) .finish_non_exhaustive() } } impl OpenAi { - /// A client against `https://api.openai.com/v1/`. + /// A client at the backend's own defaults, ready to be repointed through + /// [`Self::endpoint`] once the settings row has been read. + /// + /// Construction never depends on the API key: `llama.cpp` serves without + /// authentication, so a base URL and no key is a valid configuration + /// (DESIGN.md §15). /// /// # Errors /// /// Fails if the HTTP client cannot be constructed. - pub fn new(model: impl Into, config: OpenAiConfig) -> Result { - Self::with_base_url(model, config, DEFAULT_BASE_URL) + pub fn new(config: OpenAiConfig) -> Result { + Self::with_endpoint(config, OpenAiEndpoint::new(None, None)?) } /// Point the client at another OpenAI-compatible endpoint — `OpenRouter`, /// Groq, a self-hosted gateway, a local `llama.cpp` server, or `wiremock` - /// in tests. + /// in tests — with a fixed model. /// /// # Errors /// @@ -96,24 +103,44 @@ impl OpenAi { config: OpenAiConfig, base_url: &str, ) -> Result { + let model = model.into(); + Self::with_endpoint( + config, + OpenAiEndpoint::new(Some(base_url), Some(model.as_str()))?, + ) + } + + /// A client re-reading `endpoint` for every request, so the settings API + /// can repoint it without a restart (#220). + /// + /// # Errors + /// + /// Fails if the HTTP client cannot be constructed. + pub fn with_endpoint(config: OpenAiConfig, endpoint: OpenAiEndpoint) -> Result { let id = BackendId::new(BACKEND_NAME); let http = Client::builder().build().map_err(|err| Error::Transport { backend: id.clone(), source: Box::new(err), })?; - let base_url = base_url.parse().map_err(|err| Error::Malformed { - backend: id.clone(), - detail: format!("bad base URL: {err}"), - })?; Ok(Self { http, - base_url, + endpoint, config, - model: model.into(), id, }) } + /// The cell this backend re-reads for every request. + /// + /// Whoever constructs the backend — the daemon, at startup — hands a + /// clone of this to the settings API, which stores `openai_base_url` and + /// `openai_model` into it on every edit. Same shape as #219's + /// `Command::timeout_cell`. + #[must_use] + pub fn endpoint(&self) -> OpenAiEndpoint { + self.endpoint.clone() + } + async fn check_status(&self, response: reqwest::Response) -> Result { match response.status() { StatusCode::OK => Ok(response), @@ -160,8 +187,11 @@ impl OpenAi { detail: format!("could not encode batch: {err}"), })?; + // Re-read per call: the settings API writes this cell when the + // operator edits `openai_base_url` or `openai_model` (#220). + let endpoint = self.endpoint.read(); let body = ChatRequest { - model: &self.model, + model: &endpoint.model, messages: vec![ ChatMessage { role: "system", @@ -174,7 +204,7 @@ impl OpenAi { ], }; - let url = self + let url = endpoint .base_url .join("chat/completions") .map_err(|err| Error::Malformed { @@ -191,7 +221,7 @@ impl OpenAi { request = request.bearer_auth(api_key); } - tracing::debug!(backend = %self.id, model = %self.model, "OpenAI translate request"); + tracing::debug!(backend = %self.id, model = %endpoint.model, "OpenAI translate request"); let response = request.send().await.map_err(|err| Error::Transport { backend: self.id.clone(), source: Box::new(err), @@ -266,9 +296,12 @@ impl Backend for OpenAi { /// `models` is a read-only listing: reachable and key accepted, nothing /// spent (#200). Endpoints that need no key are probed the same way, /// just without the header. + /// + /// The base URL is read from the live cell, not from a start-up copy, so + /// the lamp judges whatever endpoint is configured right now (#220). fn probe(&self) -> ProbeFuture<'_> { Box::pin(async move { - let mut url = self.base_url.clone(); + let mut url = self.endpoint.read().base_url; url.set_path(&format!("{}/models", url.path().trim_end_matches('/'))); let mut request = self.http.get(url); if let Some(key) = &self.config.api_key { @@ -357,11 +390,25 @@ mod tests { use arr_core::Language; use super::OpenAi; + use crate::openai_endpoint::DEFAULT_MODEL; use crate::translate::Backend; use crate::translate::{language_name, strip_code_fence, system_prompt}; fn backend() -> OpenAi { - OpenAi::new("gpt-4o-mini", super::OpenAiConfig { api_key: None }).expect("client builds") + OpenAi::new(super::OpenAiConfig { api_key: None }).expect("client builds") + } + + /// #220: the backend holds the cell, not a copy — a repoint between two + /// calls is what the settings handler relies on. + #[test] + fn the_backend_follows_its_endpoint_cell() { + let backend = backend(); + let cell = backend.endpoint(); + assert_eq!(cell.model(), DEFAULT_MODEL); + cell.set(Some("http://box:8080/v1/"), Some("llama")) + .expect("repoint"); + assert_eq!(backend.endpoint.model(), "llama"); + assert_eq!(backend.endpoint.base_url(), "http://box:8080/v1/"); } #[test] diff --git a/crates/arr-subs/src/openai_endpoint.rs b/crates/arr-subs/src/openai_endpoint.rs new file mode 100644 index 0000000..466a5b9 --- /dev/null +++ b/crates/arr-subs/src/openai_endpoint.rs @@ -0,0 +1,168 @@ +//! Where the OpenAI-compatible translation backend points, and which model +//! it names (`DESIGN.md` §15, issue #220). +//! +//! Its own module, and not behind `translate-openai`, because the settings +//! surface must hold and validate these two values whether or not this build +//! compiled the backend in: the `subtitle_settings` columns exist either way, +//! and an operator who edits them on a binary without the feature should get +//! the same 422 on a base URL that does not parse. + +use std::sync::{Arc, RwLock}; + +use reqwest::Url; + +use crate::translate::{BackendId, Error, Result}; + +const BACKEND_NAME: &str = "openai"; + +/// Where the backend points when `subtitle_settings.openai_base_url` is +/// NULL — the row's own comment calls that "the backend's own default". +pub const DEFAULT_BASE_URL: &str = "https://api.openai.com/v1/"; +/// The model named when `subtitle_settings.openai_model` is NULL. Unlike the +/// base URL this is a genuine guess rather than an obvious address, so it is +/// deliberately the cheap one: an operator who cares picks another. +pub const DEFAULT_MODEL: &str = "gpt-4o-mini"; + +/// Where an [`OpenAi`](crate::openai::OpenAi) backend is pointed and which model it names, in a +/// cell it re-reads for every request (issue #220). +/// +/// Both are `subtitle_settings` rows, not bootstrap config: that backend is +/// not `OpenAI` but anything speaking the same request shape — `llama.cpp`, +/// a local gateway, a hosted provider — and which one is in use is a thing +/// the operator tries and changes (DESIGN.md §15). Whoever constructs the +/// backend hands a clone of this to the settings API, which stores the +/// edited row into it; that is the same path from database to running +/// process #219 built for the remote-command timeout. +/// +/// `None` on either side means the backend's own default — +/// [`DEFAULT_BASE_URL`] and [`DEFAULT_MODEL`]. +#[derive(Clone, Debug)] +pub struct OpenAiEndpoint { + inner: Arc>, +} + +/// One resolved endpoint, cloned out of the lock before any await. +#[derive(Clone, Debug)] +pub(crate) struct Endpoint { + pub(crate) base_url: Url, + pub(crate) model: String, +} + +/// Resolve a stored pair into a usable endpoint. A blank string is absent, +/// not an endpoint named "": the settings form sends an empty field for +/// "use the default". +fn resolve(base_url: Option<&str>, model: Option<&str>) -> Result { + let raw = base_url + .map(str::trim) + .filter(|url| !url.is_empty()) + .unwrap_or(DEFAULT_BASE_URL); + // Every path this backend builds is relative to the base, and + // `Url::join` drops the last segment of a base that does not end in a + // slash — so `http://box:8080/v1` would reach `/chat/completions` + // rather than `/v1/chat/completions`. Normalising here means the + // operator may type it either way. + let normalised = if raw.ends_with('/') { + raw.to_owned() + } else { + format!("{raw}/") + }; + let base_url: Url = normalised.parse().map_err(|err| Error::Malformed { + backend: BackendId::new(BACKEND_NAME), + detail: format!("bad base URL: {err}"), + })?; + let model = model + .map(str::trim) + .filter(|model| !model.is_empty()) + .unwrap_or(DEFAULT_MODEL) + .to_owned(); + Ok(Endpoint { base_url, model }) +} + +impl OpenAiEndpoint { + /// A cell holding `base_url` and `model`, either of which may be `None` + /// for the backend's own default. + /// + /// # Errors + /// + /// [`Error::Malformed`] when `base_url` does not parse. + pub fn new(base_url: Option<&str>, model: Option<&str>) -> Result { + Ok(Self { + inner: Arc::new(RwLock::new(resolve(base_url, model)?)), + }) + } + + /// Repoint the running backend. Validated before anything is stored, so + /// a rejected edit leaves the previous endpoint in place. + /// + /// # Errors + /// + /// [`Error::Malformed`] when `base_url` does not parse. + /// + /// # Panics + /// + /// If a previous writer panicked while holding the lock. + pub fn set(&self, base_url: Option<&str>, model: Option<&str>) -> Result<()> { + let endpoint = resolve(base_url, model)?; + *self.inner.write().expect("endpoint lock poisoned") = endpoint; + Ok(()) + } + + /// The base URL in force right now, as the backend would use it. + #[must_use] + pub fn base_url(&self) -> String { + self.read().base_url.to_string() + } + + /// The model named in force right now. + #[must_use] + pub fn model(&self) -> String { + self.read().model + } + + pub(crate) fn read(&self) -> Endpoint { + self.inner.read().expect("endpoint lock poisoned").clone() + } +} + +#[cfg(test)] +mod tests { + use super::{OpenAiEndpoint, DEFAULT_BASE_URL, DEFAULT_MODEL}; + + /// #220: NULL columns mean the backend's own default, and a base URL the + /// operator typed without a trailing slash still resolves under its own + /// path rather than at the host root. + #[test] + fn an_absent_setting_falls_back_and_a_bare_base_url_keeps_its_path() { + let endpoint = OpenAiEndpoint::new(None, None).expect("defaults resolve"); + assert_eq!(endpoint.base_url(), DEFAULT_BASE_URL); + assert_eq!(endpoint.model(), DEFAULT_MODEL); + + // An empty field is "use the default", not an endpoint named "". + endpoint.set(Some(" "), Some("")).expect("blanks resolve"); + assert_eq!(endpoint.base_url(), DEFAULT_BASE_URL); + assert_eq!(endpoint.model(), DEFAULT_MODEL); + + endpoint + .set(Some("http://127.0.0.1:8080/v1"), Some("local-model")) + .expect("a local endpoint resolves"); + assert_eq!(endpoint.base_url(), "http://127.0.0.1:8080/v1/"); + assert_eq!(endpoint.model(), "local-model"); + } + + /// A rejected edit must leave the running backend where it was, rather + /// than half-applying the model and dropping the URL. + #[test] + fn a_base_url_that_does_not_parse_is_refused_and_changes_nothing() { + let endpoint = + OpenAiEndpoint::new(Some("http://box:8080/v1"), Some("m")).expect("endpoint resolves"); + let error = endpoint + .set(Some("not a url"), Some("other")) + .expect_err("a bad base URL is refused"); + assert!( + matches!(error, crate::translate::Error::Malformed { .. }), + "got {error:?}" + ); + assert_eq!(endpoint.base_url(), "http://box:8080/v1/"); + assert_eq!(endpoint.model(), "m"); + } +} diff --git a/crates/arr-subs/tests/openai.rs b/crates/arr-subs/tests/openai.rs index 5534d5f..ce7e7f2 100644 --- a/crates/arr-subs/tests/openai.rs +++ b/crates/arr-subs/tests/openai.rs @@ -16,7 +16,7 @@ use std::time::Duration; use arr_core::Language; use arr_subs::translate::{translate, Backend, Error}; -use arr_subs::{OpenAi, OpenAiConfig}; +use arr_subs::{OpenAi, OpenAiConfig, OpenAiEndpoint}; use wiremock::matchers::{header, method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -319,9 +319,60 @@ async fn an_unsupported_target_is_refused_before_any_request() { assert!(matches!(error, Error::UnsupportedTarget { .. })); } +/// #220: an edit of `openai_base_url` / `openai_model` reaches the running +/// backend without a restart — the next batch goes to the new endpoint and +/// names the new model, with no reconstruction in between. +#[tokio::test] +async fn repointing_the_endpoint_moves_the_next_batch() { + let first = MockServer::start().await; + let second = MockServer::start().await; + mount_reply( + &first, + ResponseTemplate::new(200).set_body_string(chat_response( + r#""[{\"number\": 1, \"text\": \"do primeiro\"}]""#, + )), + ) + .await; + mount_reply( + &second, + ResponseTemplate::new(200).set_body_string(chat_response( + r#""[{\"number\": 1, \"text\": \"do segundo\"}]""#, + )), + ) + .await; + + let endpoint = OpenAiEndpoint::new(Some(&format!("{}/v1/", first.uri())), Some("first-model")) + .expect("endpoint resolves"); + let backend = + OpenAi::with_endpoint(OpenAiConfig { api_key: None }, endpoint.clone()).expect("builds"); + + let cues = vec![arr_subs::translate::BatchCue { + number: 1, + text: "from the first".to_owned(), + }]; + let batch = arr_subs::translate::Batch { + source: Language::Other("en".to_owned()), + target: Language::PortuguesePortugal, + cues: cues.clone(), + }; + let out = backend.translate(&batch).await.expect("first batch"); + assert_eq!(out[0].text, "do primeiro"); + + endpoint + .set(Some(&format!("{}/v1/", second.uri())), Some("second-model")) + .expect("repoint"); + let out = backend.translate(&batch).await.expect("second batch"); + assert_eq!(out[0].text, "do segundo"); + + let sent = second.received_requests().await.expect("requests recorded"); + let body: serde_json::Value = + serde_json::from_slice(&sent.last().expect("one request").body).expect("json body"); + assert_eq!(body["model"], "second-model"); +} + #[test] fn the_backend_answers_to_openai() { - let id = OpenAi::new("gpt-4o-mini", OpenAiConfig { api_key: None }) + let id = OpenAi::new(OpenAiConfig { api_key: None }) .expect("client builds") .id(); assert_eq!(id.as_str(), "openai"); From e8bc766d4b0b508e4a1855cf758dc43b81d32757 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:29:59 +0100 Subject: [PATCH 2/6] feat(arr): expose the OpenAI endpoint on /settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more fields on the subtitle settings row, validated on write — a base URL that does not parse is a 422 naming the field — and pushed into the cell the running backend and the health lamp both read. --- ...dbe8ae248e7e58ad8ecf635dcbe6a9d2a0289.json | 26 --- ...c446551c6afd68d85e2231742bebd215a1a4b.json | 50 +++++ ...3a9f8efefc5d400a8f94e1f4aed321284504.json} | 30 ++- ...79590e9ce55801c4b87b8db1b90f7b317225.json} | 6 +- crates/arr-api/src/state.rs | 21 ++- crates/arr-api/src/subtitle_settings.rs | 173 +++++++++++++++++- 6 files changed, 272 insertions(+), 34 deletions(-) delete mode 100644 .sqlx/query-74356a3425389d243ffdf162f4adbe8ae248e7e58ad8ecf635dcbe6a9d2a0289.json create mode 100644 .sqlx/query-7c501e25106ea8d4ad9f28ca904c446551c6afd68d85e2231742bebd215a1a4b.json rename .sqlx/{query-44a954a16f858caf8bc9ff9a3a05a1f10482c9d13e0ffd7b9f92d35bdf9cccdc.json => query-82bb36910db14d5471117285ef2d3a9f8efefc5d400a8f94e1f4aed321284504.json} (71%) rename .sqlx/{query-e4438c9d2ac3ca2bb88922c8cb16a26221efec3b1de7976d8015f61e26118cd1.json => query-eab2c4e7c21cd5d22a583c36dd2d79590e9ce55801c4b87b8db1b90f7b317225.json} (60%) diff --git a/.sqlx/query-74356a3425389d243ffdf162f4adbe8ae248e7e58ad8ecf635dcbe6a9d2a0289.json b/.sqlx/query-74356a3425389d243ffdf162f4adbe8ae248e7e58ad8ecf635dcbe6a9d2a0289.json deleted file mode 100644 index 898e8a3..0000000 --- a/.sqlx/query-74356a3425389d243ffdf162f4adbe8ae248e7e58ad8ecf635dcbe6a9d2a0289.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "db_name": "SQLite", - "query": "SELECT remote_command_timeout_seconds AS \"remote_command_timeout_seconds!: i64\"\n FROM subtitle_settings WHERE id = 1", - "describe": { - "columns": [ - { - "name": "remote_command_timeout_seconds!: i64", - "ordinal": 0, - "type_info": "Integer", - "origin": { - "Table": { - "table": "subtitle_settings", - "name": "remote_command_timeout_seconds" - } - } - } - ], - "parameters": { - "Right": 0 - }, - "nullable": [ - false - ] - }, - "hash": "74356a3425389d243ffdf162f4adbe8ae248e7e58ad8ecf635dcbe6a9d2a0289" -} diff --git a/.sqlx/query-7c501e25106ea8d4ad9f28ca904c446551c6afd68d85e2231742bebd215a1a4b.json b/.sqlx/query-7c501e25106ea8d4ad9f28ca904c446551c6afd68d85e2231742bebd215a1a4b.json new file mode 100644 index 0000000..7d9b866 --- /dev/null +++ b/.sqlx/query-7c501e25106ea8d4ad9f28ca904c446551c6afd68d85e2231742bebd215a1a4b.json @@ -0,0 +1,50 @@ +{ + "db_name": "SQLite", + "query": "SELECT remote_command_timeout_seconds AS \"remote_command_timeout_seconds!: i64\",\n openai_base_url AS \"openai_base_url: String\",\n openai_model AS \"openai_model: String\"\n FROM subtitle_settings WHERE id = 1", + "describe": { + "columns": [ + { + "name": "remote_command_timeout_seconds!: i64", + "ordinal": 0, + "type_info": "Integer", + "origin": { + "Table": { + "table": "subtitle_settings", + "name": "remote_command_timeout_seconds" + } + } + }, + { + "name": "openai_base_url: String", + "ordinal": 1, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_settings", + "name": "openai_base_url" + } + } + }, + { + "name": "openai_model: String", + "ordinal": 2, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_settings", + "name": "openai_model" + } + } + } + ], + "parameters": { + "Right": 0 + }, + "nullable": [ + false, + true, + true + ] + }, + "hash": "7c501e25106ea8d4ad9f28ca904c446551c6afd68d85e2231742bebd215a1a4b" +} diff --git a/.sqlx/query-44a954a16f858caf8bc9ff9a3a05a1f10482c9d13e0ffd7b9f92d35bdf9cccdc.json b/.sqlx/query-82bb36910db14d5471117285ef2d3a9f8efefc5d400a8f94e1f4aed321284504.json similarity index 71% rename from .sqlx/query-44a954a16f858caf8bc9ff9a3a05a1f10482c9d13e0ffd7b9f92d35bdf9cccdc.json rename to .sqlx/query-82bb36910db14d5471117285ef2d3a9f8efefc5d400a8f94e1f4aed321284504.json index c228946..4df707c 100644 --- a/.sqlx/query-44a954a16f858caf8bc9ff9a3a05a1f10482c9d13e0ffd7b9f92d35bdf9cccdc.json +++ b/.sqlx/query-82bb36910db14d5471117285ef2d3a9f8efefc5d400a8f94e1f4aed321284504.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "SELECT wanted_languages AS \"wanted_languages!: String\",\n providers_enabled AS \"providers_enabled!: String\",\n translation_engine AS \"translation_engine: String\",\n provider_daily_budgets AS \"provider_daily_budgets!: String\",\n translator_daily_budgets AS \"translator_daily_budgets!: String\",\n remote_command_timeout_seconds AS \"remote_command_timeout_seconds!: i64\"\n FROM subtitle_settings WHERE id = 1", + "query": "SELECT wanted_languages AS \"wanted_languages!: String\",\n providers_enabled AS \"providers_enabled!: String\",\n translation_engine AS \"translation_engine: String\",\n provider_daily_budgets AS \"provider_daily_budgets!: String\",\n translator_daily_budgets AS \"translator_daily_budgets!: String\",\n remote_command_timeout_seconds AS \"remote_command_timeout_seconds!: i64\",\n openai_base_url AS \"openai_base_url: String\",\n openai_model AS \"openai_model: String\"\n FROM subtitle_settings WHERE id = 1", "describe": { "columns": [ { @@ -68,6 +68,28 @@ "name": "remote_command_timeout_seconds" } } + }, + { + "name": "openai_base_url: String", + "ordinal": 6, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_settings", + "name": "openai_base_url" + } + } + }, + { + "name": "openai_model: String", + "ordinal": 7, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_settings", + "name": "openai_model" + } + } } ], "parameters": { @@ -79,8 +101,10 @@ true, false, false, - false + false, + true, + true ] }, - "hash": "44a954a16f858caf8bc9ff9a3a05a1f10482c9d13e0ffd7b9f92d35bdf9cccdc" + "hash": "82bb36910db14d5471117285ef2d3a9f8efefc5d400a8f94e1f4aed321284504" } diff --git a/.sqlx/query-e4438c9d2ac3ca2bb88922c8cb16a26221efec3b1de7976d8015f61e26118cd1.json b/.sqlx/query-eab2c4e7c21cd5d22a583c36dd2d79590e9ce55801c4b87b8db1b90f7b317225.json similarity index 60% rename from .sqlx/query-e4438c9d2ac3ca2bb88922c8cb16a26221efec3b1de7976d8015f61e26118cd1.json rename to .sqlx/query-eab2c4e7c21cd5d22a583c36dd2d79590e9ce55801c4b87b8db1b90f7b317225.json index 4a6ce32..b52433c 100644 --- a/.sqlx/query-e4438c9d2ac3ca2bb88922c8cb16a26221efec3b1de7976d8015f61e26118cd1.json +++ b/.sqlx/query-eab2c4e7c21cd5d22a583c36dd2d79590e9ce55801c4b87b8db1b90f7b317225.json @@ -1,12 +1,12 @@ { "db_name": "SQLite", - "query": "UPDATE subtitle_settings SET\n wanted_languages = ?, providers_enabled = ?, translation_engine = ?,\n provider_daily_budgets = ?, translator_daily_budgets = ?,\n remote_command_timeout_seconds = ?,\n updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now')\n WHERE id = 1", + "query": "UPDATE subtitle_settings SET\n wanted_languages = ?, providers_enabled = ?, translation_engine = ?,\n provider_daily_budgets = ?, translator_daily_budgets = ?,\n remote_command_timeout_seconds = ?,\n openai_base_url = ?, openai_model = ?,\n updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now')\n WHERE id = 1", "describe": { "columns": [], "parameters": { - "Right": 6 + "Right": 8 }, "nullable": [] }, - "hash": "e4438c9d2ac3ca2bb88922c8cb16a26221efec3b1de7976d8015f61e26118cd1" + "hash": "eab2c4e7c21cd5d22a583c36dd2d79590e9ce55801c4b87b8db1b90f7b317225" } diff --git a/crates/arr-api/src/state.rs b/crates/arr-api/src/state.rs index beae631..d64eb4a 100644 --- a/crates/arr-api/src/state.rs +++ b/crates/arr-api/src/state.rs @@ -6,7 +6,7 @@ use std::sync::{atomic::AtomicU64, Arc}; use std::time::Duration; use arr_db::Db; -use arr_subs::{Backend, Provider, Syncer}; +use arr_subs::{Backend, OpenAiEndpoint, Provider, Syncer}; use tokio::sync::mpsc; use crate::jellyfin::JellyfinClient; @@ -90,6 +90,11 @@ pub struct AppState { /// `None` unless the daemon compiled and configured that backend; the /// settings API writes it on every edit of the row. command_timeout: Option>, + /// Where the OpenAI-compatible backend points and which model it names + /// (#220). `None` unless that backend is compiled in and wired up; the + /// settings API repoints it on every edit, and the health lamp probes + /// whatever it currently holds. + openai_endpoint: Option, /// The configured `ffmpeg` binary, for the health lamps (#200). ffmpeg_binary: OsString, jellyfin: Option, @@ -167,6 +172,7 @@ impl AppState { subtitle_providers: Arc::new(Vec::new()), translation_backends: Arc::new(Vec::new()), command_timeout: None, + openai_endpoint: None, ffmpeg_binary: DEFAULT_FFMPEG_BINARY.into(), jellyfin: None, syncer: Syncer::default(), @@ -213,6 +219,15 @@ impl AppState { self } + /// Attach the cell the OpenAI-compatible backend re-reads per request + /// (#220), so an edit of `openai_base_url` or `openai_model` reaches it + /// — and the health lamp probes it — without a restart. + #[must_use] + pub fn with_openai_endpoint(mut self, endpoint: OpenAiEndpoint) -> Self { + self.openai_endpoint = Some(endpoint); + self + } + /// Attach the Jellyfin client, so a manual subtitle grab or translation /// can trigger the same library refresh import does (§7.5, §15). #[must_use] @@ -245,6 +260,10 @@ impl AppState { self.command_timeout.as_ref() } + pub(crate) fn openai_endpoint(&self) -> Option<&OpenAiEndpoint> { + self.openai_endpoint.as_ref() + } + /// Attach the `alass` binary this deployment runs (§15). Defaults to /// resolving `alass` from `PATH`. #[must_use] diff --git a/crates/arr-api/src/subtitle_settings.rs b/crates/arr-api/src/subtitle_settings.rs index 124f573..46b0622 100644 --- a/crates/arr-api/src/subtitle_settings.rs +++ b/crates/arr-api/src/subtitle_settings.rs @@ -29,6 +29,13 @@ pub struct SubtitleSettings { pub provider_daily_budgets: BTreeMap, pub translator_daily_budgets: BTreeMap, pub remote_command_timeout_seconds: u32, + /// Where the OpenAI-compatible backend points (#220). `null` means its + /// own default, `https://api.openai.com/v1/` — that backend is anything + /// speaking the shape, so a `llama.cpp` address belongs here. Not a + /// secret: the API key stays in the environment (§10). + pub openai_base_url: Option, + /// The model that backend names. `null` means its own default. + pub openai_model: Option, /// Engines DESIGN.md §15 knows about that this binary compiled in. /// `translation_engine` is always a member of this list or `null` — a /// backend whose cargo feature is missing is never selectable. @@ -48,6 +55,10 @@ pub struct SubtitleSettingsInput { #[serde(default)] pub translator_daily_budgets: BTreeMap, pub remote_command_timeout_seconds: u32, + #[serde(default)] + pub openai_base_url: Option, + #[serde(default)] + pub openai_model: Option, } impl SubtitleSettingsInput { @@ -91,6 +102,14 @@ impl SubtitleSettingsInput { if self.remote_command_timeout_seconds == 0 { return Err("remote_command_timeout_seconds: must be greater than zero".into()); } + // #220: an unparseable base URL is rejected here rather than at the + // next translation, where it would surface as an engine that has + // quietly stopped working. Validated whether or not this build + // compiled the backend in — the column exists either way. + if let Some(base_url) = blank_to_none(self.openai_base_url.as_deref()) { + arr_subs::OpenAiEndpoint::new(Some(base_url), None) + .map_err(|error| format!("openai_base_url: {error}"))?; + } Ok(()) } @@ -108,6 +127,10 @@ impl SubtitleSettingsInput { provider_daily_budgets: json(&self.provider_daily_budgets)?, translator_daily_budgets: json(&self.translator_daily_budgets)?, remote_command_timeout_seconds: i64::from(self.remote_command_timeout_seconds), + // An empty field means "use the backend's default", which is the + // NULL the migration describes — not an endpoint named "". + openai_base_url: blank_to_none(self.openai_base_url.as_deref()).map(str::to_owned), + openai_model: blank_to_none(self.openai_model.as_deref()).map(str::to_owned), }) } } @@ -120,6 +143,13 @@ struct SettingsColumns { provider_daily_budgets: String, translator_daily_budgets: String, remote_command_timeout_seconds: i64, + openai_base_url: Option, + openai_model: Option, +} + +/// A field the operator left empty is absent, not an empty setting. +fn blank_to_none(value: Option<&str>) -> Option<&str> { + value.map(str::trim).filter(|value| !value.is_empty()) } fn column( @@ -145,6 +175,8 @@ impl SettingsColumns { )?, remote_command_timeout_seconds: u32::try_from(self.remote_command_timeout_seconds) .unwrap_or(0), + openai_base_url: self.openai_base_url, + openai_model: self.openai_model, available_engines: arr_subs::compiled_engines() .into_iter() .map(str::to_owned) @@ -163,7 +195,9 @@ pub(crate) async fn load(state: &AppState) -> Result translation_engine AS "translation_engine: String", provider_daily_budgets AS "provider_daily_budgets!: String", translator_daily_budgets AS "translator_daily_budgets!: String", - remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64" + remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64", + openai_base_url AS "openai_base_url: String", + openai_model AS "openai_model: String" FROM subtitle_settings WHERE id = 1"# ) .fetch_one(pool(state)?) @@ -201,6 +235,8 @@ pub async fn update( let new_wanted = input.wanted_languages.clone(); let columns = input.into_columns()?; let timeout_seconds = columns.remote_command_timeout_seconds; + let openai_base_url = columns.openai_base_url.clone(); + let openai_model = columns.openai_model.clone(); let previous_wanted: String = sqlx::query_scalar!("SELECT wanted_languages FROM subtitle_settings WHERE id = 1") @@ -217,6 +253,7 @@ pub async fn update( wanted_languages = ?, providers_enabled = ?, translation_engine = ?, provider_daily_budgets = ?, translator_daily_budgets = ?, remote_command_timeout_seconds = ?, + openai_base_url = ?, openai_model = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = 1"#, columns.wanted_languages, @@ -225,6 +262,8 @@ pub async fn update( columns.provider_daily_budgets, columns.translator_daily_budgets, columns.remote_command_timeout_seconds, + columns.openai_base_url, + columns.openai_model, ) .execute(&mut *transaction) .await?; @@ -248,6 +287,14 @@ pub async fn update( std::sync::atomic::Ordering::Relaxed, ); } + // #220: the same path for the OpenAI-compatible backend. `validate` has + // already parsed the base URL, so this cannot fail for a reason the + // operator has not been told about. + if let Some(endpoint) = state.openai_endpoint() { + if let Err(error) = endpoint.set(openai_base_url.as_deref(), openai_model.as_deref()) { + tracing::error!(%error, "validated openai endpoint failed to apply"); + } + } Ok(Json(load(&state).await?)) } @@ -293,6 +340,31 @@ mod tests { (dir, format!("http://{address}"), timeout) } + /// The same app, with the OpenAI-compatible backend's live endpoint + /// attached — what the daemon wires up when that backend is compiled in. + async fn application_with_openai() -> (tempfile::TempDir, String, arr_subs::OpenAiEndpoint) { + let dir = tempfile::tempdir().expect("tempdir"); + let database = arr_db::Db::connect(dir.path().join("arr.db")) + .await + .expect("connect database"); + database.migrate().await.expect("migrate database"); + let endpoint = arr_subs::OpenAiEndpoint::new(None, None).expect("defaults resolve"); + let state = AppState::new(Upstreams::new( + "http://127.0.0.1:1".into(), + "http://127.0.0.1:1".into(), + )) + .expect("state") + .with_database(database) + .with_openai_endpoint(endpoint.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let address = listener.local_addr().expect("address"); + let app = router(state); + tokio::spawn(async move { axum::serve(listener, app).await.expect("serve") }); + (dir, format!("http://{address}"), endpoint) + } + /// The same app, with the underlying pool exposed so a test can seed or /// inspect rows the API surface does not read back directly. async fn application_with_pool() -> (tempfile::TempDir, String, sqlx::SqlitePool) { @@ -455,6 +527,105 @@ mod tests { ); } + /// #220: the two OpenAI-compatible endpoint fields are ordinary settings + /// — they round-trip, and an omitted or empty one reads back as `null`, + /// which the backend takes as "use your own default". + #[tokio::test] + async fn the_openai_endpoint_round_trips_and_blanks_read_back_null() { + let (_dir, base) = application().await; + let client = reqwest::Client::new(); + + let seeded: serde_json::Value = reqwest::get(format!("{base}/api/settings/subtitles")) + .await + .expect("get settings") + .json() + .await + .expect("settings json"); + assert!(seeded["openai_base_url"].is_null()); + assert!(seeded["openai_model"].is_null()); + + let mut payload = valid_input(); + payload["openai_base_url"] = serde_json::json!("http://127.0.0.1:8080/v1"); + payload["openai_model"] = serde_json::json!("qwen2.5:7b"); + let updated: serde_json::Value = client + .put(format!("{base}/api/settings/subtitles")) + .json(&payload) + .send() + .await + .expect("put settings") + .json() + .await + .expect("updated json"); + assert_eq!(updated["openai_base_url"], "http://127.0.0.1:8080/v1"); + assert_eq!(updated["openai_model"], "qwen2.5:7b"); + + // An emptied field means "back to the default", not an endpoint + // named "" — the settings form sends an empty input, not a null. + payload["openai_base_url"] = serde_json::json!(""); + payload["openai_model"] = serde_json::json!(" "); + let cleared: serde_json::Value = client + .put(format!("{base}/api/settings/subtitles")) + .json(&payload) + .send() + .await + .expect("put settings") + .json() + .await + .expect("updated json"); + assert!(cleared["openai_base_url"].is_null()); + assert!(cleared["openai_model"].is_null()); + } + + /// #220: a base URL that does not parse is a 422 naming the field, the + /// same shape `translation_engine` already rejects with. + #[tokio::test] + async fn a_base_url_that_does_not_parse_is_a_422_naming_the_field() { + let (_dir, base) = application().await; + let mut payload = valid_input(); + payload["openai_base_url"] = serde_json::json!("not a url"); + + let response = reqwest::Client::new() + .put(format!("{base}/api/settings/subtitles")) + .json(&payload) + .send() + .await + .expect("put settings"); + assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); + let body: serde_json::Value = response.json().await.expect("error json"); + assert!( + body["error"] + .as_str() + .expect("error string") + .starts_with("openai_base_url:"), + "got {body}" + ); + } + + /// #220, the point of the issue: the row alone never reaches the running + /// backend, so a PUT must repoint the cell it re-reads per request — + /// which is also what the health lamp probes. + #[tokio::test] + async fn a_put_repoints_the_live_openai_endpoint() { + let (_dir, base, endpoint) = application_with_openai().await; + assert_eq!(endpoint.base_url(), arr_subs::OPENAI_DEFAULT_BASE_URL); + assert_eq!(endpoint.model(), arr_subs::OPENAI_DEFAULT_MODEL); + + let mut payload = valid_input(); + payload["openai_base_url"] = serde_json::json!("http://127.0.0.1:8080/v1"); + payload["openai_model"] = serde_json::json!("qwen2.5:7b"); + reqwest::Client::new() + .put(format!("{base}/api/settings/subtitles")) + .json(&payload) + .send() + .await + .expect("put settings") + .error_for_status() + .expect("valid input accepted"); + + assert_eq!(endpoint.base_url(), "http://127.0.0.1:8080/v1/"); + assert_eq!(endpoint.model(), "qwen2.5:7b"); + } + /// Whether a known engine is selectable depends on which `translate-*` /// features this binary was built with, so the test asks the build rather /// than assuming. With no feature on, every engine is uncompiled and must From 5aa914f81c556a6e853679988c283ccf96ff74b5 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:29:59 +0100 Subject: [PATCH 3/6] refactor(arr): retire the OpenAI bootstrap keys #216 added translate_openai_model as an explicit stopgap; the database row replaces it, along with translate_openai_base_url. Only the API key stays in the environment. deny_unknown_fields makes a config file still carrying either one a parse error, so the move is visible rather than ignored. --- crates/arr-daemon/src/config.rs | 75 +++++++++++++------------------ crates/arr-daemon/src/main.rs | 80 ++++++++++++++++++++++----------- 2 files changed, 83 insertions(+), 72 deletions(-) diff --git a/crates/arr-daemon/src/config.rs b/crates/arr-daemon/src/config.rs index ba71794..b745446 100644 --- a/crates/arr-daemon/src/config.rs +++ b/crates/arr-daemon/src/config.rs @@ -38,9 +38,12 @@ pub const ENV_OPENSUBTITLES_USERNAME: &str = "ARR_OPENSUBTITLES_USERNAME"; pub const ENV_OPENSUBTITLES_PASSWORD: &str = "ARR_OPENSUBTITLES_PASSWORD"; // Podnapisi takes no credentials (#215): its search and download are // unauthenticated, so there is nothing here for an operator to set. +// The OpenAI-compatible backend's base URL and model are `subtitle_settings` +// rows, not bootstrap keys (#220): that backend is anything speaking the +// shape, and which endpoint is in use is something the operator changes from +// `/settings`. Only the key is here, because §10 keeps secrets out of the +// database — and an endpoint needing no key at all is valid. pub const ENV_TRANSLATE_OPENAI_API_KEY: &str = "ARR_TRANSLATE_OPENAI_API_KEY"; -pub const ENV_TRANSLATE_OPENAI_BASE_URL: &str = "ARR_TRANSLATE_OPENAI_BASE_URL"; -pub const ENV_TRANSLATE_OPENAI_MODEL: &str = "ARR_TRANSLATE_OPENAI_MODEL"; pub const ENV_TRANSLATE_DEEPL_API_KEY: &str = "ARR_TRANSLATE_DEEPL_API_KEY"; pub const ENV_TRANSLATE_DEEPL_BASE_URL: &str = "ARR_TRANSLATE_DEEPL_BASE_URL"; pub const ENV_TRANSLATE_GOOGLE_API_KEY: &str = "ARR_TRANSLATE_GOOGLE_API_KEY"; @@ -70,11 +73,6 @@ pub const DEFAULT_JELLYFIN_URL: &str = "http://localhost:8096"; pub const DEFAULT_NTFY_URL: &str = "http://localhost"; pub const DEFAULT_ALASS_PATH: &str = "alass"; pub const DEFAULT_FFMPEG_PATH: &str = "ffmpeg"; -/// #191 made the model an `OpenAi::new` argument rather than an -/// `OpenAiConfig` field; no other bootstrap key covers it, so this is that -/// key's default. -pub const DEFAULT_TRANSLATE_OPENAI_MODEL: &str = "gpt-4o-mini"; - #[derive(Debug, thiserror::Error)] pub enum ConfigError { #[error("io: {0}")] @@ -122,10 +120,6 @@ struct ConfigFile { #[serde(default)] ntfy_operator_topic: Option, #[serde(default)] - translate_openai_base_url: Option, - #[serde(default)] - translate_openai_model: Option, - #[serde(default)] translate_deepl_base_url: Option, #[serde(default)] translate_google_base_url: Option, @@ -179,8 +173,6 @@ pub struct EnvOverrides { pub opensubtitles_username: Option, pub opensubtitles_password: Option, pub translate_openai_api_key: Option, - pub translate_openai_base_url: Option, - pub translate_openai_model: Option, pub translate_deepl_api_key: Option, pub translate_deepl_base_url: Option, pub translate_google_api_key: Option, @@ -214,8 +206,6 @@ impl EnvOverrides { opensubtitles_username: std::env::var(ENV_OPENSUBTITLES_USERNAME).ok(), opensubtitles_password: std::env::var(ENV_OPENSUBTITLES_PASSWORD).ok(), translate_openai_api_key: std::env::var(ENV_TRANSLATE_OPENAI_API_KEY).ok(), - translate_openai_base_url: std::env::var(ENV_TRANSLATE_OPENAI_BASE_URL).ok(), - translate_openai_model: std::env::var(ENV_TRANSLATE_OPENAI_MODEL).ok(), translate_deepl_api_key: std::env::var(ENV_TRANSLATE_DEEPL_API_KEY).ok(), translate_deepl_base_url: std::env::var(ENV_TRANSLATE_DEEPL_BASE_URL).ok(), translate_google_api_key: std::env::var(ENV_TRANSLATE_GOOGLE_API_KEY).ok(), @@ -261,13 +251,11 @@ pub struct Config { pub opensubtitles_api_key: Option, pub opensubtitles_username: Option, pub opensubtitles_password: Option, + /// §15. `None` is valid: `llama.cpp` and a local gateway serve without + /// authentication. Where that backend points and which model it names + /// are `subtitle_settings` rows, read at start-up and re-read on every + /// edit (#220), not bootstrap config. pub translate_openai_api_key: Option, - /// §15. No default: an OpenAI-compatible endpoint has no universal - /// address the way `DeepL` or Google Translate do. - pub translate_openai_base_url: Option, - /// §15. Unlike the base URL, a model does have a usable default — - /// see [`DEFAULT_TRANSLATE_OPENAI_MODEL`]. - pub translate_openai_model: String, pub translate_deepl_api_key: Option, /// `None` means the backend's own built-in default when it lands (#192). pub translate_deepl_base_url: Option, @@ -294,8 +282,6 @@ struct SubtitleBootstrap { opensubtitles_username: Option, opensubtitles_password: Option, translate_openai_api_key: Option, - translate_openai_base_url: Option, - translate_openai_model: String, translate_deepl_api_key: Option, translate_deepl_base_url: Option, translate_google_api_key: Option, @@ -311,15 +297,6 @@ fn resolve_subtitle_bootstrap(env: &EnvOverrides, file: &ConfigFile) -> Subtitle opensubtitles_username: env.opensubtitles_username.clone(), opensubtitles_password: env.opensubtitles_password.clone(), translate_openai_api_key: env.translate_openai_api_key.clone(), - translate_openai_base_url: env - .translate_openai_base_url - .clone() - .or_else(|| file.translate_openai_base_url.clone()), - translate_openai_model: env - .translate_openai_model - .clone() - .or_else(|| file.translate_openai_model.clone()) - .unwrap_or_else(|| DEFAULT_TRANSLATE_OPENAI_MODEL.to_string()), translate_deepl_api_key: env.translate_deepl_api_key.clone(), translate_deepl_base_url: env .translate_deepl_base_url @@ -434,8 +411,6 @@ impl Config { opensubtitles_username: subtitles.opensubtitles_username, opensubtitles_password: subtitles.opensubtitles_password, translate_openai_api_key: subtitles.translate_openai_api_key, - translate_openai_base_url: subtitles.translate_openai_base_url, - translate_openai_model: subtitles.translate_openai_model, translate_deepl_api_key: subtitles.translate_deepl_api_key, translate_deepl_base_url: subtitles.translate_deepl_base_url, translate_google_api_key: subtitles.translate_google_api_key, @@ -491,10 +466,6 @@ mod tests { assert_eq!(config.jellyfin_api_key, None); assert_eq!(config.opensubtitles_api_key, None); assert_eq!(config.translate_command_template, None); - assert_eq!( - config.translate_openai_model, - DEFAULT_TRANSLATE_OPENAI_MODEL - ); assert_eq!(config.alass_path, PathBuf::from(DEFAULT_ALASS_PATH)); assert_eq!(config.ffmpeg_path, PathBuf::from(DEFAULT_FFMPEG_PATH)); } @@ -613,8 +584,6 @@ prowlarr_url = "http://prowlarr.internal:9696" std::fs::write( &path, r#" -translate_openai_base_url = "http://llm.internal/v1" -translate_openai_model = "local-model" translate_command_template = "ssh box claude -p" alass_path = "/usr/local/bin/alass" "#, @@ -625,11 +594,6 @@ alass_path = "/usr/local/bin/alass" ..EnvOverrides::default() }; let config = Config::resolve(env.clone()).unwrap(); - assert_eq!( - config.translate_openai_base_url.as_deref(), - Some("http://llm.internal/v1") - ); - assert_eq!(config.translate_openai_model, "local-model"); assert_eq!( config.translate_command_template.as_deref(), Some("ssh box claude -p") @@ -646,6 +610,27 @@ alass_path = "/usr/local/bin/alass" assert_eq!(config.ffmpeg_path, PathBuf::from("/opt/bin/ffmpeg")); } + /// #220 retired `translate_openai_base_url` and `translate_openai_model` + /// — both are `subtitle_settings` rows now. `deny_unknown_fields` turns + /// a config file still carrying them into a parse error, so an operator + /// who upgrades sees the move rather than a setting silently ignored. + #[test] + fn the_retired_openai_endpoint_keys_are_a_parse_error() { + for key in ["translate_openai_base_url", "translate_openai_model"] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("arr.toml"); + std::fs::write(&path, format!("{key} = \"x\"\n")).unwrap(); + let env = EnvOverrides { + config_file: Some(path.to_string_lossy().into_owned()), + ..EnvOverrides::default() + }; + assert!( + matches!(Config::resolve(env), Err(ConfigError::TomlDecode(_))), + "{key} must no longer be accepted" + ); + } + } + #[test] fn tmdb_url_is_an_env_only_seam() { let config = Config::resolve(EnvOverrides::default()).unwrap(); diff --git a/crates/arr-daemon/src/main.rs b/crates/arr-daemon/src/main.rs index 55c45e1..882dfde 100644 --- a/crates/arr-daemon/src/main.rs +++ b/crates/arr-daemon/src/main.rs @@ -141,6 +141,9 @@ fn api_state( if let Some(timeout) = &translators.command_timeout { state = state.with_command_timeout(Arc::clone(timeout)); } + if let Some(endpoint) = &translators.openai_endpoint { + state = state.with_openai_endpoint(endpoint.clone()); + } Ok(state) } @@ -165,7 +168,7 @@ async fn run() -> Result<(), Error> { // lane must see the same backends, or the command translator's live // timeout cell (#219) would fork. let translators = translation_backends(&config); - seed_command_timeout(&database, &translators).await?; + seed_translator_settings(&database, &translators).await?; let (reconcile, manual_grab, manual_tv) = reconcile_loop( &database, &config, @@ -264,27 +267,45 @@ async fn run() -> Result<(), Error> { } } -/// Seed the command translator's live timeout from the settings row, so a -/// restart does not fall back to the compiled-in default until the next -/// settings edit (issue #219). The row is guaranteed to exist and to carry a -/// positive value — migration `0025` seeds it and the column CHECK enforces -/// it. -async fn seed_command_timeout(database: &Db, translators: &Translators) -> Result<(), Error> { - let Some(cell) = &translators.command_timeout else { +/// Seed the translators' live settings from the row, so a restart does not +/// fall back to compiled-in defaults until the next settings edit (issues +/// #219 and #220): the command translator's timeout, and where the +/// OpenAI-compatible backend points plus which model it names. +/// +/// The row is guaranteed to exist — migration `0025` seeds it — and the +/// timeout is guaranteed positive by its column CHECK. The two `OpenAI` +/// columns are nullable, and NULL means the backend's own default. +async fn seed_translator_settings(database: &Db, translators: &Translators) -> Result<(), Error> { + if translators.command_timeout.is_none() && translators.openai_endpoint.is_none() { return Ok(()); - }; - let seconds: i64 = sqlx::query_scalar!( - r#"SELECT remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64" + } + let row = sqlx::query!( + r#"SELECT remote_command_timeout_seconds AS "remote_command_timeout_seconds!: i64", + openai_base_url AS "openai_base_url: String", + openai_model AS "openai_model: String" FROM subtitle_settings WHERE id = 1"# ) .fetch_one(database.pool()) .await?; - cell.store( - u64::try_from(seconds) - .unwrap_or(u64::MAX) - .saturating_mul(1_000), - std::sync::atomic::Ordering::Relaxed, - ); + if let Some(cell) = &translators.command_timeout { + cell.store( + u64::try_from(row.remote_command_timeout_seconds) + .unwrap_or(u64::MAX) + .saturating_mul(1_000), + std::sync::atomic::Ordering::Relaxed, + ); + } + if let Some(endpoint) = &translators.openai_endpoint { + // A row that does not parse must not stop the daemon booting: the + // API validates on write, so this only fires for a hand-edited + // database. The backend stays at its default and the health lamp + // says so. + if let Err(error) = + endpoint.set(row.openai_base_url.as_deref(), row.openai_model.as_deref()) + { + tracing::warn!(%error, "stored OpenAI endpoint is unusable; keeping the default"); + } + } Ok(()) } @@ -647,6 +668,10 @@ fn subtitle_providers( struct Translators { backends: Vec>, command_timeout: Option>, + /// The OpenAI-compatible backend's live endpoint (#220), when that + /// backend is compiled in. Its base URL and model are database rows, so + /// this rides along the same way the command timeout does. + openai_endpoint: Option, } #[cfg_attr( @@ -661,22 +686,22 @@ struct Translators { fn translation_backends(config: &Config) -> Translators { let mut backends: Vec> = Vec::new(); let mut command_timeout: Option> = None; + let mut openai_endpoint: Option = None; #[cfg(feature = "translate-openai")] { + // Built at the backend's own defaults and repointed from the + // settings row a moment later (#220). Never gated on the API key: + // `llama.cpp` serves without authentication, so a base URL and no + // key is a valid configuration (DESIGN.md §15). let openai_config = arr_subs::OpenAiConfig { api_key: config.translate_openai_api_key.clone(), }; - let backend = match &config.translate_openai_base_url { - Some(base_url) => arr_subs::OpenAi::with_base_url( - config.translate_openai_model.clone(), - openai_config, - base_url, - ), - None => arr_subs::OpenAi::new(config.translate_openai_model.clone(), openai_config), - }; - match backend { - Ok(backend) => backends.push(std::sync::Arc::new(backend)), + match arr_subs::OpenAi::new(openai_config) { + Ok(backend) => { + openai_endpoint = Some(backend.endpoint()); + backends.push(std::sync::Arc::new(backend)); + } Err(error) => tracing::warn!(%error, "OpenAI-compatible translator not available"), } } @@ -737,5 +762,6 @@ fn translation_backends(config: &Config) -> Translators { Translators { backends, command_timeout, + openai_endpoint, } } From e467bd47fb38868753d17ad15c2fa6ddfaa53f04 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:29:59 +0100 Subject: [PATCH 4/6] feat(arr): add the OpenAI endpoint fields to /settings Shown only when openai is among available_engines. Placeholders say the base URL is optional and what a local one looks like. --- web/src/settings.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/web/src/settings.ts b/web/src/settings.ts index 949aacd..7abd344 100644 --- a/web/src/settings.ts +++ b/web/src/settings.ts @@ -46,6 +46,15 @@ export interface SubtitleSettings { provider_daily_budgets: Record; translator_daily_budgets: Record; remote_command_timeout_seconds: number; + /** + * Where the OpenAI-compatible backend points and which model it names + * (§15, #220). `null` on either means the backend's own default — that + * backend is anything speaking the shape, `llama.cpp` included, so both + * are settings rather than deployment facts. Neither is a secret; the API + * key stays in the environment. + */ + openai_base_url: string | null; + openai_model: string | null; /** Engines this binary compiled in — `translation_engine` is always one of these, or null. */ available_engines: string[]; } @@ -815,6 +824,20 @@ export function settingsMain(views: { hide: () => void }[], goHome: () => void): const timeout = numberControl(settings.remote_command_timeout_seconds); grid.append(field("remote command timeout (s)", timeout)); + // #220: only meaningful when this build compiled the backend in, and the + // engine list is exactly that fact. Placeholders carry the two things + // the labels cannot: that the base URL is optional, and what a local one + // looks like. + const hasOpenAi = settings.available_engines.includes("openai"); + const openAiBaseUrl = textControl( + settings.openai_base_url ?? "", + "optional — e.g. http://127.0.0.1:8080/v1", + ); + const openAiModel = textControl(settings.openai_model ?? "", "optional — e.g. gpt-4o-mini"); + if (hasOpenAi) { + grid.append(field("openai base url", openAiBaseUrl), field("openai model", openAiModel)); + } + panel.append(grid); if (!hasEngines) { panel.append( @@ -908,6 +931,10 @@ export function settingsMain(views: { hide: () => void }[], goHome: () => void): provider_daily_budgets: providerBudgets, translator_daily_budgets: translatorBudgets, remote_command_timeout_seconds: Number(timeout.value), + // An emptied field is "use the backend's default", which the API + // stores as NULL. + openai_base_url: hasOpenAi ? openAiBaseUrl.value.trim() || null : settings.openai_base_url, + openai_model: hasOpenAi ? openAiModel.value.trim() || null : settings.openai_model, }; void saveSubtitleSettings(input).then((outcome) => { if (outcome.ok) { From abad9b9cfd31fade87e788922f081f9d95d74c77 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:33:47 +0100 Subject: [PATCH 5/6] fix(arr): name the field, not a reply, in the 422 The backend's Malformed Display describes an answer that came back wrong. Nothing is sent while validating a settings write, so only the reason belongs in the message. --- crates/arr-api/src/subtitle_settings.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/arr-api/src/subtitle_settings.rs b/crates/arr-api/src/subtitle_settings.rs index 46b0622..659e714 100644 --- a/crates/arr-api/src/subtitle_settings.rs +++ b/crates/arr-api/src/subtitle_settings.rs @@ -107,8 +107,16 @@ impl SubtitleSettingsInput { // quietly stopped working. Validated whether or not this build // compiled the backend in — the column exists either way. if let Some(base_url) = blank_to_none(self.openai_base_url.as_deref()) { - arr_subs::OpenAiEndpoint::new(Some(base_url), None) - .map_err(|error| format!("openai_base_url: {error}"))?; + arr_subs::OpenAiEndpoint::new(Some(base_url), None).map_err(|error| { + // The backend's own Display talks about a reply that came + // back wrong; here nothing was ever sent, so only the + // reason belongs in the message. + let reason = match &error { + arr_subs::translate::Error::Malformed { detail, .. } => detail.clone(), + other => other.to_string(), + }; + format!("openai_base_url: {reason}") + })?; } Ok(()) } @@ -592,12 +600,9 @@ mod tests { .expect("put settings"); assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); let body: serde_json::Value = response.json().await.expect("error json"); - assert!( - body["error"] - .as_str() - .expect("error string") - .starts_with("openai_base_url:"), - "got {body}" + assert_eq!( + body["error"], "openai_base_url: bad base URL: relative URL without a base", + "the message names the field and the reason, not a reply that never came" ); } From a45d8ff86a0bd9cd624f225cd169e0a3df9e24d8 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 08:33:47 +0100 Subject: [PATCH 6/6] style(arr): give the OpenAI endpoint its own row Five fields wrapped the model onto a line of its own and clipped the base URL placeholder. Paired under one label, the way the translator budgets already are, and the base URL gets the width it needs. --- web/src/settings.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/web/src/settings.ts b/web/src/settings.ts index 7abd344..2bfd61a 100644 --- a/web/src/settings.ts +++ b/web/src/settings.ts @@ -824,19 +824,14 @@ export function settingsMain(views: { hide: () => void }[], goHome: () => void): const timeout = numberControl(settings.remote_command_timeout_seconds); grid.append(field("remote command timeout (s)", timeout)); - // #220: only meaningful when this build compiled the backend in, and the - // engine list is exactly that fact. Placeholders carry the two things - // the labels cannot: that the base URL is optional, and what a local one - // looks like. const hasOpenAi = settings.available_engines.includes("openai"); const openAiBaseUrl = textControl( settings.openai_base_url ?? "", "optional — e.g. http://127.0.0.1:8080/v1", ); const openAiModel = textControl(settings.openai_model ?? "", "optional — e.g. gpt-4o-mini"); - if (hasOpenAi) { - grid.append(field("openai base url", openAiBaseUrl), field("openai model", openAiModel)); - } + openAiBaseUrl.setAttribute("aria-label", "openai base url"); + openAiModel.setAttribute("aria-label", "openai model"); panel.append(grid); if (!hasEngines) { @@ -849,6 +844,17 @@ export function settingsMain(views: { hide: () => void }[], goHome: () => void): ); } + // Their own labelled row rather than two more cells in the grid above: + // paired, they belong to one engine, and a base URL needs the width. + if (hasOpenAi) { + panel.append( + el("p", "field-label readout dim", "openai-compatible endpoint — blank means its default"), + ); + const openAiGrid = el("div", "form-grid"); + openAiGrid.append(field("base url", openAiBaseUrl), field("model", openAiModel)); + panel.append(openAiGrid); + } + const translatorInputs: Record = {}; if (hasEngines) { panel.append(el("p", "field-label readout dim", "translator daily budgets"));