altair823-org/kebab
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 46 Wiki Activity

review(p6-2): 회차 1 지적 반영

Browse Source 
- crates/kebab-config/src/lib.rs:
  • `OcrCfg.endpoint: String` (\"\" sentinel) → `Option<String>` 으로 교체.
    `#[serde(default)]` 적용. `KEBAB_IMAGE_OCR_ENDPOINT=\"\"` (빈 값) 도
    None 으로 매핑하는 분기 추가.
  • 신규 회귀 테스트 `image_ocr_endpoint_empty_env_value_is_none`.
- crates/kebab-parse-image/src/ocr.rs:
  • `OllamaVisionOcr::new` 의 endpoint fallback 로직을 새 `Option<String>`
    스키마에 맞춰 정리 (`as_deref` + match).
  • `OllamaGenerateResponse` 의 dead `_other: HashMap<String, Value>` 필드
    제거. `serde_json::Value` import 도 같이 정리.
  • `OllamaGenerateRequest.images: Vec<&'a str>` → `[&'a str; 1]`
    (호출당 vec! 알로케이션 제거, multi-image 는 OcrEngine trait 가
    단일 이미지를 받으므로 OOS).
  • `downscale_to_long_edge` 단일-디코드로 리팩터. PNG passthrough
    hot path 보존 (header sniff 만으로 분기), 그 외 모든 경로는
    decode 1회 + (필요 시) resize + PNG re-encode 1회로 통일.
  • `pub fn max_pixels(&self) -> u32` accessor 추가 — clamp 결과
    검증 용 (단순 inspector).
- crates/kebab-parse-image/tests/ocr.rs:
  • `cfg_for_endpoint` / 통합 테스트가 `Some(endpoint)` 형태로 갱신.
  • `from_parts_clamps_max_pixels_into_legal_range` 가 새 accessor
    로 실제 클램프 결과 (256 / 4096 / 1024) 를 검증하도록 강화.
  • 통합 테스트가 폰트 부재 시 panic 대신 skip 하도록 분기.
- crates/kebab-parse-image/tests/common/mod.rs:
  • `hello_world_png` 가 `anyhow::Result<Vec<u8>>` 반환하도록 변경.
    expect(\"DejaVu Sans Bold required\") 메시지를 \"only the opt-in
    OCR integration fixture needs this font\" 로 의도 명확화.

cargo test -p kebab-parse-image — 28 pass + 1 ignored.
cargo test -p kebab-config — 21 pass (+1 회귀).
cargo clippy --workspace --all-targets -- -D warnings — pass.

Reviewer-suggested workspace.dependencies 통합 (reqwest / base64) 은
P6-3 와 함께 처리할 수 있도록 follow-up 으로 두고 본 PR scope 에서
제외 (회차 1 본문에서 명시).
This commit is contained in:
altair823
2026-05-02 05:45:25 +00:00
parent 4ed5536c92
commit e869710d82
4 changed files with 125 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
crates/kebab-config/src/lib.rs
View File

@@ -136,10 +136,11 @@ pub struct OcrCfg {
/// Model id passed to the engine (e.g. `"gemma4:e4b"` for
/// Ollama-vision).
pub model: String,
/// HTTP endpoint for the OCR engine. Empty string means "fall back
/// to `models.llm.endpoint`" — convenient when the same Ollama
/// host serves both LLM and vision.
pub endpoint: String,
/// HTTP endpoint for the OCR engine. `None` (or a missing key in
/// TOML) means "fall back to `models.llm.endpoint`" — convenient
/// when the same Ollama host serves both LLM and vision.
#[serde(default)]
pub endpoint: Option<String>,
/// BCP-47 language hints (e.g. `["eng", "kor"]`). The adapter
/// renders them into the prompt; the LLM honours them probabilistically.
pub languages: Vec<String>,
@@ -154,7 +155,7 @@ impl OcrCfg {
enabled: false,
engine: "ollama-vision".to_string(),
model: "gemma4:e4b".to_string(),
endpoint: String::new(),
endpoint: None,
languages: vec!["eng".to_string(), "kor".to_string()],
max_pixels: 1600,
}
@@ -393,7 +394,15 @@ impl Config {
}
"KEBAB_IMAGE_OCR_ENGINE" => self.image.ocr.engine = v.clone(),
"KEBAB_IMAGE_OCR_MODEL" => self.image.ocr.model = v.clone(),
"KEBAB_IMAGE_OCR_ENDPOINT" => self.image.ocr.endpoint = v.clone(),
"KEBAB_IMAGE_OCR_ENDPOINT" => {
// Empty env value is treated the same as "fall back
// to models.llm.endpoint" — i.e. set None.
self.image.ocr.endpoint = if v.is_empty() {
None
} else {
Some(v.clone())
};
}
"KEBAB_IMAGE_OCR_LANGUAGES" => {
// Comma-separated list, e.g. "eng,kor".
self.image.ocr.languages = v
@@ -578,6 +587,8 @@ mod tests {
"KEBAB_IMAGE_OCR_ENDPOINT".to_string(),
"http://192.168.0.47:11434".to_string(),
);
// Empty env value should map to None (= fall back to llm.endpoint).
// We exercise that branch in a separate test.
env.insert(
"KEBAB_IMAGE_OCR_LANGUAGES".to_string(),
"eng, kor, jpn".to_string(),
@@ -586,7 +597,10 @@ mod tests {
let c = Config::defaults().apply_env(&env);
assert!(c.image.ocr.enabled);
assert_eq!(c.image.ocr.model, "gemma4:31b");
assert_eq!(c.image.ocr.endpoint, "http://192.168.0.47:11434");
assert_eq!(
c.image.ocr.endpoint.as_deref(),
Some("http://192.168.0.47:11434")
);
assert_eq!(c.image.ocr.languages, vec!["eng", "kor", "jpn"]);
assert_eq!(c.image.ocr.max_pixels, 2048);
}
@@ -594,6 +608,17 @@ mod tests {
/// Pre-P6 config files don't have an `[image]` section. The
/// `#[serde(default)]` attribute on `Config::image` must let those
/// files load with `ImageCfg::defaults()` instead of erroring.
/// `KEBAB_IMAGE_OCR_ENDPOINT=""` (empty value) should map to `None`
/// rather than to `Some("")` so the fallback to `models.llm.endpoint`
/// kicks in. Covers the env-equivalent of a missing TOML key.
#[test]
fn image_ocr_endpoint_empty_env_value_is_none() {
let mut env = HashMap::new();
env.insert("KEBAB_IMAGE_OCR_ENDPOINT".to_string(), String::new());
let c = Config::defaults().apply_env(&env);
assert_eq!(c.image.ocr.endpoint, None);
}
#[test]
fn pre_p6_config_without_image_section_loads_with_defaults() {
let toml_text = r#"

87
crates/kebab-parse-image/src/ocr.rs
View File

@@ -34,7 +34,6 @@ use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
use image::{ImageFormat, ImageReader};
use kebab_core::{ImageRefBlock, Lang, OcrRegion, OcrText, ProvenanceEvent, ProvenanceKind};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use time::OffsetDateTime;
/// Engine name written into `OcrText.engine` for the Ollama-vision adapter.
@@ -135,10 +134,9 @@ impl OllamaVisionOcr {
/// happens inside [`OcrEngine::recognize`].
pub fn new(config: &kebab_config::Config) -> Result<Self> {
let ocr = &config.image.ocr;
let endpoint = if ocr.endpoint.is_empty() {
config.models.llm.endpoint.clone()
} else {
ocr.endpoint.clone()
let endpoint = match ocr.endpoint.as_deref() {
Some(s) if !s.is_empty() => s.to_string(),
_ => config.models.llm.endpoint.clone(),
};
if endpoint.is_empty() {
anyhow::bail!(
@@ -184,6 +182,14 @@ impl OllamaVisionOcr {
})
}
/// Effective `max_pixels` after the `[MIN_LONG_EDGE, MAX_LONG_EDGE]`
/// clamp. Exposed so tests can verify the clamp result without
/// reaching into the private field; production callers don't need
/// it.
pub fn max_pixels(&self) -> u32 {
self.max_pixels
}
fn build_prompt(&self, lang_hint: Option<&Lang>) -> String {
let langs = if self.languages.is_empty() {
"any".to_string()
@@ -228,7 +234,7 @@ impl OcrEngine for OllamaVisionOcr {
let body = OllamaGenerateRequest {
model: &self.model,
prompt: &prompt,
images: vec![&b64],
images: [b64.as_str()],
stream: false,
options: OllamaOptions {
temperature: 0.0,
@@ -297,9 +303,11 @@ impl OcrEngine for OllamaVisionOcr {
/// Decode `bytes`, downscale so the long edge is at most `max_long_edge`,
/// and re-encode as PNG. Returns `(png_bytes, final_w, final_h)`.
///
/// Bypasses encode work when the source already fits — we simply pass
/// the bytes through. PNG re-encode is only paid when downscaling is
/// actually needed.
/// PNG sources that already fit the cap are passthrough (zero decodes,
/// just a `Vec` clone). Every other path decodes the image exactly
/// once: the cheap header sniff peeks at the format / dimensions before
/// committing to a decode, so non-PNG passthrough and downscale share
/// the same `decode → optionally resize → re-encode` tail.
fn downscale_to_long_edge(bytes: &[u8], max_long_edge: u32) -> Result<(Vec<u8>, u32, u32)> {
let reader = ImageReader::new(Cursor::new(bytes))
.with_guessed_format()
@@ -310,40 +318,39 @@ fn downscale_to_long_edge(bytes: &[u8], max_long_edge: u32) -> Result<(Vec<u8>,
.context("reading image dimensions for OCR")?;
let long = w.max(h);
if long <= max_long_edge {
// Source fits — avoid the round-trip through `image::DynamicImage`.
// Re-encode only when the source isn't already PNG, since the
// wire format we send Ollama is PNG.
return match format {
Some(ImageFormat::Png) => Ok((bytes.to_vec(), w, h)),
_ => {
let img = ImageReader::new(Cursor::new(bytes))
.with_guessed_format()
.context("re-reading image for PNG re-encode")?
.decode()
.context("decoding image for PNG re-encode")?;
let mut out = Cursor::new(Vec::new());
img.write_to(&mut out, ImageFormat::Png)
.context("re-encoding image as PNG")?;
Ok((out.into_inner(), w, h))
}
};
// Hot path — PNG within budget already matches the wire format we
// send Ollama, so we ship the bytes verbatim without paying for a
// decode + re-encode round-trip.
if long <= max_long_edge && format == Some(ImageFormat::Png) {
return Ok((bytes.to_vec(), w, h));
}
let scale = max_long_edge as f32 / long as f32;
let new_w = ((w as f32) * scale).round().max(1.0) as u32;
let new_h = ((h as f32) * scale).round().max(1.0) as u32;
// Every remaining branch needs the pixels — either to re-encode as
// PNG (non-PNG within budget) or to resize first (over budget).
// One decode covers both.
let img = ImageReader::new(Cursor::new(bytes))
.with_guessed_format()
.context("re-reading image for downscale")?
.context("re-reading image for OCR decode")?
.decode()
.context("decoding image for downscale")?;
let resized = img.resize_exact(new_w, new_h, image::imageops::FilterType::Triangle);
.context("decoding image for OCR")?;
let (final_w, final_h, final_img) = if long <= max_long_edge {
(w, h, img)
} else {
let scale = max_long_edge as f32 / long as f32;
let new_w = ((w as f32) * scale).round().max(1.0) as u32;
let new_h = ((h as f32) * scale).round().max(1.0) as u32;
let resized =
img.resize_exact(new_w, new_h, image::imageops::FilterType::Triangle);
(new_w, new_h, resized)
};
let mut out = Cursor::new(Vec::new());
resized
final_img
.write_to(&mut out, ImageFormat::Png)
.context("encoding downscaled image as PNG")?;
Ok((out.into_inner(), new_w, new_h))
.context("encoding image as PNG for OCR")?;
Ok((out.into_inner(), final_w, final_h))
}
fn truncate(s: &str, n: usize) -> String {
@@ -361,7 +368,11 @@ fn truncate(s: &str, n: usize) -> String {
struct OllamaGenerateRequest<'a> {
model: &'a str,
prompt: &'a str,
images: Vec<&'a str>,
/// Always exactly one image — the `OcrEngine` trait takes a single
/// `&[u8]`, so multi-image batching is out of scope until a future
/// trait extension. Fixed-size array avoids the `vec![]`
/// allocation per call.
images: [&'a str; 1],
stream: bool,
options: OllamaOptions,
}
@@ -378,8 +389,6 @@ struct OllamaGenerateResponse {
response: Option<String>,
#[serde(default)]
error: Option<String>,
#[serde(flatten)]
_other: std::collections::HashMap<String, Value>,
}
#[cfg(test)]

25
crates/kebab-parse-image/tests/common/mod.rs
View File

@@ -60,18 +60,23 @@ pub fn large_blue_4000x3000_png() -> Vec<u8> {
/// `ocr_integration_real_ollama_transcribes_text` integration test —
/// regular hermetic tests never call it.
///
/// Renders with a font shipped by `dejavu` (path under
/// `/usr/share/fonts/truetype/dejavu/`) — common across most Linux dev
/// boxes. Falls back to a tiny built-in glyph map if the font is
/// missing so the helper compiles even without DejaVu installed.
pub fn hello_world_png() -> Vec<u8> {
/// Returns `Err` (not panic) if the DejaVu Sans Bold font is missing
/// from the standard Linux path, so dev boxes without the font can
/// gracefully skip the integration test rather than crashing the
/// process.
pub fn hello_world_png() -> anyhow::Result<Vec<u8>> {
use ab_glyph::{Font, FontRef, ScaleFont};
use anyhow::Context;
let mut img: ImageBuffer<Rgb<u8>, _> =
ImageBuffer::from_fn(400, 100, |_, _| Rgb([255, 255, 255]));
let font_bytes = std::fs::read("/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf")
.expect("DejaVu Sans Bold required for OCR integration fixture");
let font = FontRef::try_from_slice(&font_bytes).expect("font parses");
let font_path = "/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf";
let font_bytes = std::fs::read(font_path).with_context(|| {
format!(
"{font_path} not found — only the opt-in OCR integration fixture needs this font"
)
})?;
let font = FontRef::try_from_slice(&font_bytes).context("DejaVu font parses")?;
let scaled = font.as_scaled(40.0);
let text = "Hello World 2026";
let mut x = 10.0_f32;
@@ -93,8 +98,8 @@ pub fn hello_world_png() -> Vec<u8> {
}
let mut buf = Cursor::new(Vec::new());
img.write_to(&mut buf, image::ImageFormat::Png)
.expect("encoding hello-world PNG must not fail");
buf.into_inner()
.context("encoding hello-world PNG")?;
Ok(buf.into_inner())
}
/// JPEG with embedded EXIF APP1 segment carrying GPS + Make + Model +

40
crates/kebab-parse-image/tests/ocr.rs
View File

@@ -20,7 +20,7 @@ use crate::common::red_100x50_png;
fn cfg_for_endpoint(endpoint: &str) -> Config {
let mut cfg = Config::defaults();
cfg.image.ocr.endpoint = endpoint.to_string();
cfg.image.ocr.endpoint = Some(endpoint.to_string());
cfg.image.ocr.model = "gemma4:e4b".to_string();
cfg.image.ocr.languages = vec!["eng".to_string(), "kor".to_string()];
cfg.image.ocr.max_pixels = 1024;
@@ -321,13 +321,26 @@ async fn ocr_downscales_large_image_before_sending() {
#[test]
fn from_parts_clamps_max_pixels_into_legal_range() {
// Below MIN_LONG_EDGE — bumped up to the floor.
let too_small = OllamaVisionOcr::from_parts("http://x", "m", vec![], 10).unwrap();
let too_big = OllamaVisionOcr::from_parts("http://x", "m", vec![], 99_999).unwrap();
// We can't read the private field directly, but engine_version is
// observable; assert the engine constructed at all (clamp didn't
// panic) and run a synthetic prompt.
assert_eq!(too_small.engine_name(), "ollama-vision");
assert_eq!(too_big.engine_name(), "ollama-vision");
assert_eq!(
too_small.max_pixels(),
256,
"max_pixels must be raised to MIN_LONG_EDGE"
);
// Above MAX_LONG_EDGE — capped at the ceiling.
let too_big =
OllamaVisionOcr::from_parts("http://x", "m", vec![], 99_999).unwrap();
assert_eq!(
too_big.max_pixels(),
4096,
"max_pixels must be capped at MAX_LONG_EDGE"
);
// Inside the legal range — pass through untouched.
let in_range = OllamaVisionOcr::from_parts("http://x", "m", vec![], 1024).unwrap();
assert_eq!(in_range.max_pixels(), 1024);
}
// ── Integration test against real Ollama (opt-in) ────────────────────────
@@ -355,11 +368,18 @@ async fn ocr_integration_real_ollama_transcribes_text() {
let model =
std::env::var("KEBAB_IMAGE_OCR_MODEL").unwrap_or_else(|_| "gemma4:e4b".to_string());
// Generate a fixture with known text.
let bytes = common::hello_world_png();
// Generate a fixture with known text. If the DejaVu font is
// missing from this dev box, skip rather than crash.
let bytes = match common::hello_world_png() {
Ok(b) => b,
Err(e) => {
eprintln!("skipping ocr_integration: {e:#}");
return;
}
};
let cfg = {
let mut c = Config::defaults();
c.image.ocr.endpoint = endpoint;
c.image.ocr.endpoint = Some(endpoint);
c.image.ocr.model = model;
c.image.ocr.max_pixels = 1024;
c