diff --git a/backend/Cargo.lock b/backend/Cargo.lock index ee26703..abfda4e 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.13" +version = "0.87.14" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 6954dc6..3cb524b 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.13" +version = "0.87.14" edition = "2021" default-run = "mangalord" diff --git a/backend/src/analysis/vision.rs b/backend/src/analysis/vision.rs index bda7c09..6e5388d 100644 --- a/backend/src/analysis/vision.rs +++ b/backend/src/analysis/vision.rs @@ -77,14 +77,33 @@ enum PreparedAnalysis { /// JPEG encode. Returns `Ok(Undecodable)` (never `Err`) when the bytes /// don't parse, so the caller can fall through to the raw-pass-through /// behaviour. +/// +/// **Diagnosability.** The five fallback paths below all return +/// `Undecodable`, which the caller treats as "send raw bytes". That's +/// correct semantically — a server can sometimes salvage a broken +/// page — but it also makes a silent JPEG-encoder regression +/// indistinguishable from "the page was just garbage." Emit a `warn` +/// on each fallback so an operator can grep for "vision prep fell +/// through" and tell the two apart. fn prepare_analysis(image: &[u8], params: SliceParams) -> PreparedAnalysis { let Some(img) = image::load_from_memory(image).ok() else { + tracing::warn!( + bytes = image.len(), + "vision prep fell through to Undecodable: image::load_from_memory failed" + ); return PreparedAnalysis::Undecodable; }; match plan_slices(img.width(), img.height(), ¶ms) { Plan::Single { .. } => match render_whole(&img, params.max_pixels) { Some(jpeg) => PreparedAnalysis::Single { jpeg }, - None => PreparedAnalysis::Undecodable, + None => { + tracing::warn!( + width = img.width(), + height = img.height(), + "vision prep fell through to Undecodable: render_whole on Single plan failed" + ); + PreparedAnalysis::Undecodable + } }, Plan::Sliced { width, height, bands } => { let work = if width == img.width() && height == img.height() { @@ -98,11 +117,23 @@ fn prepare_analysis(image: &[u8], params: SliceParams) -> PreparedAnalysis { // A failed slice falls back to the undecodable path — // the server then gets one combined call instead of N // broken slice calls. + tracing::warn!( + y0 = *y0, + y1 = *y1, + width, + height, + "vision prep fell through to Undecodable: render_slice failed" + ); return PreparedAnalysis::Undecodable; }; slices.push((*y0, *y1, jpeg)); } let Some(whole) = render_whole(&img, params.max_pixels) else { + tracing::warn!( + width = img.width(), + height = img.height(), + "vision prep fell through to Undecodable: render_whole on Sliced plan failed" + ); return PreparedAnalysis::Undecodable; }; PreparedAnalysis::Sliced { slices, whole } @@ -1140,4 +1171,79 @@ mod tests { other => panic!("expected Undecodable, got {:?}", std::mem::discriminant(&other)), } } + + /// Pin the contract that the 0.87.5 fix is about: the heavy image + /// work in `analyze()` MUST go through `spawn_blocking` so the tokio + /// runtime stays responsive. If a refactor accidentally inlined + /// `prepare_analysis` back into the async function, this test would + /// catch it. + /// + /// How we detect it: run `analyze()` on a `current_thread` runtime + /// (one worker thread). Schedule a concurrent counter task that + /// ticks every 5ms. The image is large enough that `prepare_analysis` + /// takes meaningfully longer than the tick interval. If prep is on + /// `spawn_blocking`, the worker thread stays free to drive the + /// counter and we observe several ticks. If prep is inlined onto + /// the runtime, the worker is starved and the counter cannot + /// advance during the prep window. + /// + /// Reqwest's connection refused returns within milliseconds, so the + /// HTTP error path doesn't dominate the timing — the prep phase + /// owns the wall-clock here. + #[tokio::test(flavor = "current_thread", start_paused = false)] + async fn analyze_dispatches_image_prep_off_runtime() { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + use std::time::Duration; + + // VisionClient pointed at a closed local port. Reqwest fails the + // POST immediately (connection refused), but the prep work runs + // before the POST is even built. + let mut cfg = AnalysisConfig::default(); + cfg.endpoint = "http://127.0.0.1:1/v1/chat/completions".into(); + cfg.model = "stub".into(); + cfg.request_timeout = Duration::from_secs(1); + // Tune slice geometry to match the test image's shape. + cfg.max_pixels = 1_000_000; + cfg.min_slice_height = 100; + cfg.tall_aspect_threshold = 1.8; + let http = reqwest::Client::builder() + .timeout(cfg.request_timeout) + .no_proxy() + .build() + .unwrap(); + let client = VisionClient::new(http, &cfg); + + // ~6 MP image so decode + JPEG encode comfortably exceeds the + // 5 ms tick interval on every realistic CI runner. + let big = jpeg_of(2000, 3000); + + // Counter that ticks at 5ms. If the runtime is starved, this + // task makes no progress. + let ticks = Arc::new(AtomicUsize::new(0)); + let ticks_c = Arc::clone(&ticks); + let ticker = tokio::spawn(async move { + let mut interval = tokio::time::interval(Duration::from_millis(5)); + interval.tick().await; // discard immediate first tick + loop { + interval.tick().await; + ticks_c.fetch_add(1, Ordering::Relaxed); + } + }); + + let _ = client.analyze(&big, "image/jpeg").await; + ticker.abort(); + let observed = ticks.load(Ordering::Relaxed); + + // A blocked runtime would yield 0 ticks during the prep window; + // a free runtime yields many. 2 is a generous floor that + // tolerates slow CI hardware without sacrificing the regression + // signal. + assert!( + observed >= 2, + "runtime ticked only {observed} times during analyze() — \ + image prep is likely blocking the runtime instead of using \ + spawn_blocking" + ); + } } diff --git a/frontend/package.json b/frontend/package.json index e8445c2..611b815 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.13", + "version": "0.87.14", "private": true, "type": "module", "scripts": {