refactor(arr): compute translator billing per call

Backend::characters_billed() polled a cumulative counter that races
under concurrent closes and can't attribute cost to one call. Drop it
in favour of the caller counting source characters before it sends
anything, which #197's budget needs anyway.
This commit is contained in:
Miguel Palhas
2026-08-25 02:40:02 +01:00
parent 7b4cff1874
commit f49507b7d1
3 changed files with 12 additions and 75 deletions
+2 -32
View File
@@ -10,13 +10,13 @@
//! One request translates a whole batch: the texts go out as a JSON array
//! and come back as a same-length array of translations, order preserved,
//! so cue numbering never leaves [`crate::translate`]. Characters sent are
//! what `DeepL` bills, so they are counted and reported for #197's budget.
//! what `DeepL` bills; #197's budget counts those itself before this ever
//! runs, rather than polling this client for them afterwards.
//!
//! The auth key arrives from bootstrap config or environment per §10 —
//! `ARR_TRANSLATE_DEEPL_API_KEY`, plumbed through `arr-daemon`'s config —
//! never a database row.
use std::sync::atomic::{AtomicU64, Ordering};
use std::time::Duration;
use reqwest::{Client, StatusCode, Url};
@@ -64,7 +64,6 @@ pub struct DeepL {
base_url: Url,
config: DeepLConfig,
id: BackendId,
billed: AtomicU64,
}
// Hand-written: the key must never reach a log line.
@@ -109,7 +108,6 @@ impl DeepL {
base_url: url,
config,
id: BackendId::new("deepl"),
billed: AtomicU64::new(0),
})
}
@@ -186,10 +184,6 @@ impl Backend for DeepL {
deep_code(target, false).is_some()
}
fn characters_billed(&self) -> u64 {
self.billed.load(Ordering::Relaxed)
}
fn translate<'a>(&'a self, batch: &'a Batch) -> TranslateFuture<'a> {
Box::pin(async move {
let texts: Vec<String> = batch.cues.iter().map(|cue| cue.text.clone()).collect();
@@ -277,11 +271,6 @@ impl Backend for DeepL {
});
}
self.billed.fetch_add(
texts.iter().map(|text| text.chars().count() as u64).sum(),
Ordering::Relaxed,
);
Ok(parsed
.translations
.into_iter()
@@ -455,25 +444,6 @@ mod tests {
assert_eq!(body["source_lang"], "PT");
}
#[tokio::test]
async fn characters_sent_are_reported_as_billed() {
let server = MockServer::start().await;
mount_translate(&server, 200, &reply_body(&[r#""x""#])).await;
let client = deepl(server.uri());
assert_eq!(client.characters_billed(), 0);
client
.translate(&batch(
Language::Other("en".to_owned()),
Language::PortugueseBrazil,
&["four"],
))
.await
.expect("the mock answers");
assert_eq!(client.characters_billed(), 4);
}
#[tokio::test]
async fn a_wrong_translation_count_is_a_mismatch_not_a_subtitle() {
let server = MockServer::start().await;
+3 -34
View File
@@ -13,15 +13,14 @@
//!
//! * replies HTML-escape characters even with `format: "text"`
//! (`&quot;`, `&#39;`), so they are decoded before returning;
//! * per-character usage is not echoed back, so characters sent are counted
//! on this side and reported for #197's budget.
//! * per-character usage is not echoed back. #197's budget does not need it
//! from here either — it counts source characters itself before this ever
//! runs.
//!
//! The API key arrives from bootstrap config or environment per §10 —
//! `ARR_TRANSLATE_GOOGLE_API_KEY`, plumbed through `arr-daemon`'s config —
//! never a database row.
use std::sync::atomic::{AtomicU64, Ordering};
use reqwest::{Client, StatusCode, Url};
use serde::{Deserialize, Serialize};
@@ -58,7 +57,6 @@ pub struct Google {
base_url: Url,
config: GoogleConfig,
id: BackendId,
billed: AtomicU64,
}
// Hand-written: the key must never reach a log line.
@@ -97,7 +95,6 @@ impl Google {
base_url: url,
config,
id: BackendId::new("google"),
billed: AtomicU64::new(0),
})
}
}
@@ -200,10 +197,6 @@ impl Backend for Google {
google_code(target).is_some()
}
fn characters_billed(&self) -> u64 {
self.billed.load(Ordering::Relaxed)
}
fn translate<'a>(&'a self, batch: &'a Batch) -> TranslateFuture<'a> {
Box::pin(async move {
let texts: Vec<String> = batch.cues.iter().map(|cue| cue.text.clone()).collect();
@@ -285,11 +278,6 @@ impl Backend for Google {
});
}
self.billed.fetch_add(
texts.iter().map(|text| text.chars().count() as u64).sum(),
Ordering::Relaxed,
);
Ok(parsed
.data
.translations
@@ -523,25 +511,6 @@ mod tests {
assert_eq!(body["source"], "pt");
}
#[tokio::test]
async fn characters_sent_are_reported_as_billed() {
let server = MockServer::start().await;
mount_translate(&server, 200, &reply_body(&[r#""x""#])).await;
let client = google(server.uri());
assert_eq!(client.characters_billed(), 0);
client
.translate(&batch(
Language::Other("en".to_owned()),
Language::PortugueseBrazil,
&["four"],
))
.await
.expect("the mock answers");
assert_eq!(client.characters_billed(), 4);
}
#[tokio::test]
async fn a_wrong_translation_count_is_a_mismatch_not_a_subtitle() {
let server = MockServer::start().await;
+7 -9
View File
@@ -114,6 +114,13 @@ pub type TranslateFuture<'a> =
/// A backend translates one batch and reports what happened. It does not
/// chunk, does not validate its own replies — [`translate`] does both, so a
/// backend cannot skip them — and never sees a timestamp.
///
/// It also does not report what it billed. §15's translator budget (#197)
/// spends against source characters sent, which the caller already knows
/// before any batch goes out — a cumulative counter polled off the backend
/// cannot attribute one call's cost when several run concurrently on the
/// same instance (§8 runs up to four closes at once), so nothing here tracks
/// it.
pub trait Backend: fmt::Debug + Send + Sync {
/// The name this backend answers to in settings and the UI.
fn id(&self) -> BackendId;
@@ -128,15 +135,6 @@ pub trait Backend: fmt::Debug + Send + Sync {
BATCH_CHAR_BUDGET
}
/// Characters this backend has billed so far, cumulative over its life.
///
/// Translators charge per character (DESIGN.md §15); the daily budget
/// (#197) spends against what was actually billed rather than an
/// estimate of it. A backend that cannot say reports zero.
fn characters_billed(&self) -> u64 {
0
}
/// Translate one batch.
///
/// The reply is expected to carry every cue number the batch carried,