Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 84 additions & 3 deletions crates/multimodal/src/vision/preprocessor_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,14 @@ impl PreProcessorConfig {
.and_then(|v| v.as_u64())
.map(|v| v as usize);
}
// Also extract Kimi-specific limits into the extra map
// so processors can read them via get_extra()
for key in ["in_patch_limit", "patch_limit_on_one_side"] {
// Also extract Kimi-specific limits and the transparency settings into
// the extra map so processors can read them via get_extra()
for key in [
"in_patch_limit",
"patch_limit_on_one_side",
"transparent_bg_config",
"transparent_bg_fill_stage",
] {
if !config.extra.contains_key(key) {
if let Some(v) = media_cfg.get(key) {
config.extra.insert(key.to_string(), v.clone());
Expand Down Expand Up @@ -434,6 +439,20 @@ impl PreProcessorConfig {
.and_then(|v| serde_json::from_value(v.clone()).ok())
}

/// The checkpoint's alpha-flattening behavior, if it declares any.
///
/// `None` means the checkpoint asks for no compositing, matching the
/// reference's behavior when `transparent_bg_config` is absent. The fill
/// stage is only meaningful alongside a config, so it is read here rather
/// than exposed on its own.
pub fn transparent_bg(&self) -> Option<transforms::TransparentBg> {
let config = self.get_extra::<transforms::TransparentBgConfig>("transparent_bg_config")?;
let stage = self
.get_extra::<transforms::TransparentBgFillStage>("transparent_bg_fill_stage")
.unwrap_or_default();
Some(transforms::TransparentBg { config, stage })
}
Comment on lines +442 to +454

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

A malformed transparent_bg_config silently degrades to alpha-drop.

get_extra discards deserialization errors, so a checkpoint that ships "pattern": "chess_board" (or any type mismatch) yields None here, and K3 then renders transparent regions as solid black with no signal anywhere. Log when the key is present but unparseable so the misconfiguration is diagnosable.

♻️ Surface the parse failure
     pub fn transparent_bg(&self) -> Option<transforms::TransparentBg> {
-        let config = self.get_extra::<transforms::TransparentBgConfig>("transparent_bg_config")?;
+        let raw = self.extra.get("transparent_bg_config")?;
+        let config = match serde_json::from_value::<transforms::TransparentBgConfig>(raw.clone()) {
+            Ok(config) => config,
+            Err(e) => {
+                tracing::warn!(error = %e, "unparseable transparent_bg_config; alpha will be dropped");
+                return None;
+            }
+        };
         let stage = self
             .get_extra::<transforms::TransparentBgFillStage>("transparent_bg_fill_stage")
             .unwrap_or_default();
         Some(transforms::TransparentBg { config, stage })
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// The checkpoint's alpha-flattening behavior, if it declares any.
///
/// `None` means the checkpoint asks for no compositing, matching the
/// reference's behavior when `transparent_bg_config` is absent. The fill
/// stage is only meaningful alongside a config, so it is read here rather
/// than exposed on its own.
pub fn transparent_bg(&self) -> Option<transforms::TransparentBg> {
let config = self.get_extra::<transforms::TransparentBgConfig>("transparent_bg_config")?;
let stage = self
.get_extra::<transforms::TransparentBgFillStage>("transparent_bg_fill_stage")
.unwrap_or_default();
Some(transforms::TransparentBg { config, stage })
}
/// The checkpoint's alpha-flattening behavior, if it declares any.
///
/// `None` means the checkpoint asks for no compositing, matching the
/// reference's behavior when `transparent_bg_config` is absent. The fill
/// stage is only meaningful alongside a config, so it is read here rather
/// than exposed on its own.
pub fn transparent_bg(&self) -> Option<transforms::TransparentBg> {
let raw = self.extra.get("transparent_bg_config")?;
let config = match serde_json::from_value::<transforms::TransparentBgConfig>(raw.clone()) {
Ok(config) => config,
Err(e) => {
tracing::warn!(error = %e, "unparseable transparent_bg_config; alpha will be dropped");
return None;
}
};
let stage = self
.get_extra::<transforms::TransparentBgFillStage>("transparent_bg_fill_stage")
.unwrap_or_default();
Some(transforms::TransparentBg { config, stage })
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/multimodal/src/vision/preprocessor_config.rs` around lines 442 - 454,
Update transparent_bg to distinguish an absent transparent_bg_config from a
present but unparseable value, and log the deserialization error when parsing
fails. Preserve returning None only when the key is absent, while valid configs
should continue constructing TransparentBg with the existing stage behavior.


// Common default values
pub const CLIP_MEAN: [f64; 3] = [0.48145466, 0.4578275, 0.40821073];
pub const CLIP_STD: [f64; 3] = [0.26862954, 0.26130258, 0.27577711];
Expand Down Expand Up @@ -601,5 +620,67 @@ mod tests {

assert_eq!(config.get_patch_size(0), 14);
assert_eq!(config.merge_size, Some(2));

// K2.5 declares no transparency handling.
assert_eq!(config.transparent_bg(), None);
}

#[test]
fn test_parse_kimi_k3_transparency_settings() {
// Excerpt from moonshotai/Kimi-K3's preprocessor_config.json.
let json = r#"{
"media_proc_cfg": {
"in_patch_limit": 65536,
"patch_size": 14,
"merge_kernel_size": 2,
"patch_limit_on_one_side": 512,
"transparent_bg_config": {
"pattern": "chessboard",
"chessboard_square_size": 16,
"chessboard_square_on_top_left": true,
"chessboard_white_value": 255,
"chessboard_gray_value": 200
},
"transparent_bg_fill_stage": "after_resize"
}
}"#;

let config = PreProcessorConfig::from_json(json).unwrap();

assert_eq!(config.get_extra::<usize>("in_patch_limit"), Some(65536));
assert_eq!(
config.get_extra::<usize>("patch_limit_on_one_side"),
Some(512)
);

let bg = config
.transparent_bg()
.expect("transparent_bg_config lifted out of media_proc_cfg");
assert_eq!(
bg.config.pattern,
transforms::TransparentBgPattern::Chessboard
);
assert_eq!(bg.config.chessboard_square_size, 16);
assert!(bg.config.chessboard_square_on_top_left);
assert_eq!(bg.config.chessboard_white_value, 255);
assert_eq!(bg.config.chessboard_gray_value, 200);
assert_eq!(bg.stage, transforms::TransparentBgFillStage::AfterResize);
}

#[test]
fn test_transparency_fill_stage_defaults_without_explicit_key() {
// The reference falls back to "before_resize" when the key is absent.
let json = r#"{
"media_proc_cfg": {
"transparent_bg_config": { "pattern": "white" }
}
}"#;

let bg = PreProcessorConfig::from_json(json)
.unwrap()
.transparent_bg()
.expect("config present");
assert_eq!(bg.config.pattern, transforms::TransparentBgPattern::White);
assert_eq!(bg.stage, transforms::TransparentBgFillStage::BeforeResize);
}
}
9 changes: 6 additions & 3 deletions crates/multimodal/src/vision/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ impl VisionProcessorRegistry {
Box::new(super::processors::Llama4VisionProcessor::new()),
);

// Register Kimi-K2.5 Vision (also used by Kimi-K3, same MoonViT stack)
// Register Kimi-K2.5 Vision
registry.register(
"kimi-k2",
Box::new(super::processors::KimiK25Processor::new()),
Expand All @@ -334,13 +334,16 @@ impl VisionProcessorRegistry {
"kimi_k2",
Box::new(super::processors::KimiK25Processor::new()),
);

// Register Kimi-K3 Vision. Shares K2.5's MoonViT stack but keeps its own
// processor: the reference ships a separate class whose parameters differ.
registry.register(
"kimi-k3",
Box::new(super::processors::KimiK25Processor::new()),
Box::new(super::processors::KimiK3Processor::new()),
);
registry.register(
"kimi_k3",
Box::new(super::processors::KimiK25Processor::new()),
Box::new(super::processors::KimiK3Processor::new()),
);

registry
Expand Down
Loading
Loading