Skip to content

Commit a322c37

Browse files
authored
feat(mofa-slides): list_styles tool + discourage auto_layout + skill-output-aligned paths (#62)
* feat(mofa-slides): add mofa_list_styles tool and discourage auto_layout default * New mofa_list_styles tool scans find_styles_dir for any skill, returns live catalog (name, description, variants, tags). Lets the LLM see what's actually installed instead of relying on a hardcoded table that drifts every release. * plugin_slides no longer silently swaps in nb-pro.toml when a style file is missing — it bails with the available-style list. The silent fallback was masking deployment drift and producing purple output for styles like lingnan when the deployed copy was stale. * SKILL.md rewritten: * Mode 2 (manual texts) is now the explicit default for "editable PPT". * auto_layout (Mode 3) has a "when NOT to use this" section explaining it's slow, expensive, and produces output that usually needs heavy human cleanup. Only appropriate when user explicitly asks for VQA or for PDF-to-PPTX (Mode 4). * Hardcoded 17-style table replaced with a directive to call mofa_list_styles (deployed dir actually has 20 styles, validating the drift). * manifest.json: declare mofa_list_styles; rewrite style + auto_layout descriptions to match. * Style accessors variant_names() / default_variant_name() / list_style_names() added to back the new tool. * Bump to 0.5.0 (was out of sync between SKILL.md 0.4.2 and manifest 0.4.5). * docs(mofa-slides): SKILL.md output paths align with octos skill-output rebind The Octos host rebinds plugin work_dir to <workspace>/skill-output/, so an LLM that emits `out: "skill-output/mofa-slides-XX/deck.pptx"` gets a double-prefix (`<workspace>/skill-output/skill-output/...`) that the workspace contract validator never finds. Inside an Octos slides workspace (the normal case), the LLM should pass workspace-relative `slides/<slug>/output/deck.pptx` — the host rebind places it at `<workspace>/skill-output/slides/<slug>/output/`, the workspace contract picks it up via files_to_send, and delivery is automatic. Standalone use (mofa CLI outside Octos) keeps a unique-per-request subdir but drops the skill-output prefix. * docs(mofa-slides): Mode 4 example uses workspace-relative source_image The Mode 4 (PDF-to-PPTX) example used `source_image: "skill-output/pdf-pages/..."` which contradicts the rule added two commits ago ("never prefix skill-output/ yourself"). Inside an Octos slides workspace the host does NOT rebind input paths (only `out` / `slide_dir`), so the example was still functionally valid — but the contradiction would train the LLM to use skill-output prefixes elsewhere and trip the double-prefix failure on output args. Switch the example to a slides-project-relative input path (slides/<slug>/assets/pdf-pages/...) and explicitly note that input-path semantics differ from output-path semantics. Codex review of #62 — MINOR.
1 parent fe25b69 commit a322c37

6 files changed

Lines changed: 283 additions & 66 deletions

File tree

mofa-cli/src/main.rs

Lines changed: 124 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ fn run_plugin(tool_name: &str, cancel: &std::sync::atomic::AtomicBool) -> Result
353353
plugin_infographic(&args, &mofa_root, &cfg).map(|s| (s.into(), serde_json::Value::Null))
354354
}
355355
"mofa_video" => plugin_video(&args, &mofa_root, &cfg).map(|s| (s.into(), serde_json::Value::Null)),
356+
"mofa_list_styles" => {
357+
plugin_list_styles(&args, &mofa_root).map(|(s, v)| (s.into(), v))
358+
}
356359
_ => Err(eyre::eyre!("unknown tool: {tool_name}")),
357360
};
358361

@@ -525,7 +528,10 @@ fn plugin_slides(
525528
}
526529

527530
// Check workspace styles first (agent-created), then built-in styles.
528-
// If neither exists, fall back to nb-pro so generation never fails on style.
531+
// If neither exists, bail loudly so the caller can pick a real style —
532+
// a silent swap to a different theme hides deployment drift (e.g. an
533+
// older skill snapshot missing a style) and produces output that looks
534+
// like the LLM picked the wrong color scheme on purpose.
529535
let style_filename = format!("{style_name}.toml");
530536
let builtin_dir = find_styles_dir(mofa_root, "slides");
531537
let cwd_style = std::env::current_dir()
@@ -538,8 +544,17 @@ fn plugin_slides(
538544
} else if builtin_style.exists() {
539545
builtin_style
540546
} else {
541-
eprintln!("style '{}' not found, falling back to nb-pro", style_name);
542-
builtin_dir.join("nb-pro.toml")
547+
let available = style::list_style_names(&builtin_dir);
548+
let list = if available.is_empty() {
549+
format!("(none found in {})", builtin_dir.display())
550+
} else {
551+
available.join(", ")
552+
};
553+
eyre::bail!(
554+
"style '{style_name}' not found under {}. Available: {list}. \
555+
Call the `mofa_list_styles` tool to inspect variants and descriptions.",
556+
builtin_dir.display()
557+
);
543558
};
544559
let loaded_style = style::load_style(&style_file)?;
545560

@@ -902,6 +917,112 @@ fn plugin_video(
902917
))
903918
}
904919

920+
/// Scan the styles dir for a given skill and return the live catalog.
921+
/// Reads each `*.toml`, extracts `[meta]` + variant names, and returns a JSON object.
922+
/// The LLM should call this BEFORE picking a style so it never asks for one
923+
/// that isn't on the deployed copy.
924+
fn plugin_list_styles(
925+
args: &serde_json::Value,
926+
mofa_root: &std::path::Path,
927+
) -> Result<(String, serde_json::Value)> {
928+
let skill = args
929+
.get("skill")
930+
.and_then(|v| v.as_str())
931+
.unwrap_or("slides");
932+
let styles_dir = find_styles_dir(mofa_root, skill);
933+
let names = style::list_style_names(&styles_dir);
934+
935+
let mut styles_json: Vec<serde_json::Value> = Vec::with_capacity(names.len());
936+
for name in &names {
937+
let path = styles_dir.join(format!("{name}.toml"));
938+
let entry = match style::load_style(&path) {
939+
Ok(s) => {
940+
let meta = s
941+
.meta
942+
.clone()
943+
.unwrap_or(toml::Value::Table(Default::default()));
944+
// Surface common meta fields as first-class JSON keys; everything else
945+
// (tags, category, etc.) ends up under `meta`.
946+
let display_name = meta
947+
.get("display_name")
948+
.and_then(|v| v.as_str())
949+
.unwrap_or(name)
950+
.to_string();
951+
let description = meta
952+
.get("description")
953+
.and_then(|v| v.as_str())
954+
.unwrap_or("")
955+
.to_string();
956+
let category = meta
957+
.get("category")
958+
.and_then(|v| v.as_str())
959+
.map(str::to_string);
960+
let tags: Vec<String> = meta
961+
.get("tags")
962+
.and_then(|v| v.as_array())
963+
.map(|arr| {
964+
arr.iter()
965+
.filter_map(|v| v.as_str().map(str::to_string))
966+
.collect()
967+
})
968+
.unwrap_or_default();
969+
serde_json::json!({
970+
"name": name,
971+
"display_name": display_name,
972+
"description": description,
973+
"category": category,
974+
"tags": tags,
975+
"variants": s.variant_names(),
976+
"default_variant": s.default_variant_name(),
977+
})
978+
}
979+
Err(e) => serde_json::json!({
980+
"name": name,
981+
"error": format!("{e:#}"),
982+
}),
983+
};
984+
styles_json.push(entry);
985+
}
986+
987+
let summary = serde_json::json!({
988+
"skill": skill,
989+
"styles_dir": styles_dir.to_string_lossy(),
990+
"count": names.len(),
991+
"styles": styles_json,
992+
});
993+
994+
let human = if names.is_empty() {
995+
format!(
996+
"No styles found under {} (skill={skill})",
997+
styles_dir.display()
998+
)
999+
} else {
1000+
let mut lines = vec![format!(
1001+
"{} {skill} styles available (from {}):",
1002+
names.len(),
1003+
styles_dir.display()
1004+
)];
1005+
for s in &summary["styles"].as_array().cloned().unwrap_or_default() {
1006+
let name = s.get("name").and_then(|v| v.as_str()).unwrap_or("?");
1007+
let display = s.get("display_name").and_then(|v| v.as_str()).unwrap_or("");
1008+
let variants = s
1009+
.get("variants")
1010+
.and_then(|v| v.as_array())
1011+
.map(|arr| {
1012+
arr.iter()
1013+
.filter_map(|v| v.as_str())
1014+
.collect::<Vec<_>>()
1015+
.join(", ")
1016+
})
1017+
.unwrap_or_default();
1018+
lines.push(format!(" • {name} — {display} [variants: {variants}]"));
1019+
}
1020+
lines.join("\n")
1021+
};
1022+
1023+
Ok((human, summary))
1024+
}
1025+
9051026
fn main() -> Result<()> {
9061027
color_eyre::install()?;
9071028

mofa-cli/src/openai.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,7 @@ impl OpenAIImageClient {
8888
{
8989
Ok(resp) => {
9090
if let Ok(data) = resp.json::<Value>() {
91-
if let Some(b64) =
92-
data.pointer("/data/0/b64_json").and_then(|v| v.as_str())
91+
if let Some(b64) = data.pointer("/data/0/b64_json").and_then(|v| v.as_str())
9392
{
9493
if let Ok(bytes) = base64::Engine::decode(
9594
&base64::engine::general_purpose::STANDARD,

mofa-cli/src/pipeline/slides.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ use crate::config::MofaConfig;
44
use crate::dashscope::DashscopeClient;
55
use crate::deepseek_ocr::DeepSeekOcrClient;
66
use crate::gemini::{BatchImageRequest, GeminiClient};
7-
use crate::openai::OpenAIImageClient;
87
use crate::layout::{
98
extract_text_layout, extract_text_layout_deepseek, refine_text_layout, ANTI_LEAK_RULES,
109
NO_TEXT_INSTRUCTION, SH, SW,
1110
};
11+
use crate::openai::OpenAIImageClient;
1212
use crate::pptx::{self, ImageOverlay, SlideData, TextOverlay};
1313
use crate::style::Style;
1414
use eyre::Result;
@@ -152,7 +152,14 @@ fn generate_image(
152152
if model.starts_with("gpt-image") {
153153
if let Some(ref oa) = openai {
154154
return oa
155-
.gen_image(prompt, out_file, image_size, Some("16:9"), Some(model), Some(label))
155+
.gen_image(
156+
prompt,
157+
out_file,
158+
image_size,
159+
Some("16:9"),
160+
Some(model),
161+
Some(label),
162+
)
156163
.ok()
157164
.flatten()
158165
.inspect(|path| {

mofa-cli/src/style.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,37 @@ impl Style {
2323
.map(|s| s.as_str())
2424
.unwrap_or("")
2525
}
26+
27+
/// Variant names declared in this style (excludes the synthetic `default` key).
28+
pub fn variant_names(&self) -> Vec<String> {
29+
let mut names: Vec<String> = self.variants.keys().cloned().collect();
30+
names.sort();
31+
names
32+
}
33+
34+
/// Name of the default variant (the one used when no `style` is specified).
35+
pub fn default_variant_name(&self) -> &str {
36+
&self.default_variant
37+
}
38+
}
39+
40+
/// Scan a styles directory and return the list of style names (file stems of `*.toml`)
41+
/// without parsing the prompt bodies. Returns an empty vec if the dir is missing.
42+
pub fn list_style_names(dir: &Path) -> Vec<String> {
43+
let mut names = Vec::new();
44+
let Ok(entries) = std::fs::read_dir(dir) else {
45+
return names;
46+
};
47+
for entry in entries.flatten() {
48+
let path = entry.path();
49+
if path.extension().map(|e| e == "toml").unwrap_or(false) {
50+
if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) {
51+
names.push(stem.to_string());
52+
}
53+
}
54+
}
55+
names.sort();
56+
names
2657
}
2758

2859
/// Load a single TOML style file.

0 commit comments

Comments
 (0)