fix(analysis-vision): pin spawn_blocking dispatch + warn on undecodable fallback (0.87.14)
Two 0.87.5 follow-ups: 1. Pin spawn_blocking dispatch: new current_thread runtime test runs analyze() on a 6 MP image while a counter task ticks every 5 ms. Without spawn_blocking the counter is starved (0 ticks); with it the runtime stays responsive (≥2 ticks). Mutation-confirmed. 2. Warn on Undecodable fallback: five fallback paths in prepare_analysis now emit a tracing::warn! with dimensions so a JPEG encoder regression isn't indistinguishable from "the page was just garbage." Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.87.13"
|
||||
version = "0.87.14"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.87.13"
|
||||
version = "0.87.14"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user