diff --git a/crates/op-orchestrator/src/compact_skills.rs b/crates/op-orchestrator/src/compact_skills.rs index e87aa4542..c915e2f09 100644 --- a/crates/op-orchestrator/src/compact_skills.rs +++ b/crates/op-orchestrator/src/compact_skills.rs @@ -16,22 +16,26 @@ use crate::model_profile::ModelTier; -/// Filter a resolved skill list according to retry parameters. +/// Filter a resolved skill list according to retry + content parameters. /// -/// Mirrors the `minimalSkills` / `reducedComplexity` branches in -/// `executeSubAgent` (orchestrator-sub-agent.ts:428-453) and the -/// `retryAllowed` set in `compactSubAgentSkills` -/// (orchestrator-sub-agent-compact.ts:53-72). +/// Port of the `minimalSkills` branch in `executeSubAgent` +/// (orchestrator-sub-agent.ts:428-431) followed by `compactSubAgentSkills` +/// (orchestrator-sub-agent-compact.ts:4-76), which TS calls unconditionally +/// on every sub-agent prompt. /// /// # Arguments -/// * `skills` — The full resolved skill list from `resolve_skills`. -/// * `tier` — The model's capability tier. -/// * `minimal_skills` — When `true`, keep only `schema` + `jsonl-format`. -/// * `reduced_complexity`— When `true` AND `tier == Basic`, keep the +/// * `skills` — The full resolved skill list from `resolve_skills`. +/// * `tier` — The model's capability tier. +/// * `is_mobile_screen` — Whether the plan is a full mobile screen. +/// * `has_explicit_style_guide` — A style guide or design.md is in effect. +/// * `minimal_skills` — When `true`, keep only `schema` + `jsonl-format`. +/// * `reduced_complexity` — When `true` AND `tier == Basic`, narrow to the /// `retryAllowed` 8-skill set (excludes `elements`). pub fn apply_skill_filter( skills: Vec, tier: ModelTier, + is_mobile_screen: bool, + has_explicit_style_guide: bool, minimal_skills: bool, reduced_complexity: bool, ) -> Vec { @@ -48,29 +52,93 @@ pub fn apply_skill_filter( .collect(); } - if reduced_complexity && tier == ModelTier::Basic { - // Reduced-complexity retry for Basic tier: the retryAllowed set. - // Verbatim port from orchestrator-sub-agent-compact.ts:54-71. - // `elements` is deliberately OMITTED — see comment in TS source. - const RETRY_ALLOWED: &[&str] = &[ - "schema", - "jsonl-format-simplified", - "layout", - "text-rules", - "mobile-app", - "style-defaults", - "design-md", - "variables", - ]; - return skills - .into_iter() - .filter(|s| RETRY_ALLOWED.contains(&s.skill_name())) - .collect(); + compact_subagent_skills( + skills, + tier, + is_mobile_screen, + has_explicit_style_guide, + reduced_complexity, + ) +} + +/// Port of `compactSubAgentSkills` (orchestrator-sub-agent-compact.ts:4-76): +/// a content-aware base filter for ALL tiers, then a `jsonl-format` dedup, +/// then a Basic-tier allow-set (further narrowed on reduced-complexity). +fn compact_subagent_skills( + skills: Vec, + tier: ModelTier, + is_mobile_screen: bool, + has_explicit_style_guide: bool, + reduced_complexity: bool, +) -> Vec { + // Base filter (all tiers): mobile/desktop + explicit-style-guide drops. + let mut next: Vec = skills + .into_iter() + .filter(|s| { + let name = s.skill_name(); + if is_mobile_screen + && (name == "landing-page" || name == "copywriting" || name == "anti-slop") + { + return false; + } + if !is_mobile_screen && name == "mobile-app" { + return false; + } + if has_explicit_style_guide && name == "design-system" { + return false; + } + true + }) + .collect(); + + // When the simplified JSONL format is present, drop the verbose one so a + // Basic-tier model doesn't carry both (orchestrator-sub-agent-compact.ts:24-28). + let has_simplified = next + .iter() + .any(|s| s.skill_name() == "jsonl-format-simplified"); + if has_simplified { + next.retain(|s| s.skill_name() != "jsonl-format"); } - // Standard / Full tier with reduced_complexity, or no filtering requested: - // return unchanged. - skills + if tier == ModelTier::Basic { + // Basic-tier allow-set (orchestrator-sub-agent-compact.ts:31-52). + // `elements` is included; it's already gated off at the resolve layer + // when the element-tools flag is false, so keeping it here is a no-op + // in that case and required when the flag is on. + const ALLOWED: &[&str] = &[ + "schema", + "jsonl-format-simplified", + "jsonl-format", + "layout", + "overflow", + "text-rules", + "variables", + "design-md", + "mobile-app", + "icon-catalog", + "style-defaults", + "elements", + ]; + next.retain(|s| ALLOWED.contains(&s.skill_name())); + + if reduced_complexity { + // Reduced-complexity retry kernel (orchestrator-sub-agent-compact.ts:54-71). + // `elements` deliberately OMITTED — the retry wants the smallest prompt. + const RETRY_ALLOWED: &[&str] = &[ + "schema", + "jsonl-format-simplified", + "layout", + "text-rules", + "mobile-app", + "style-defaults", + "design-md", + "variables", + ]; + next.retain(|s| RETRY_ALLOWED.contains(&s.skill_name())); + } + } + + next } /// Minimal trait so the filter works over any struct that exposes a skill @@ -108,6 +176,16 @@ mod tests { skills.iter().map(|s| s.skill_name()).collect() } + // Convenience: the common non-mobile, no-style-guide call shape. + fn filter( + skills: Vec, + tier: ModelTier, + minimal: bool, + reduced: bool, + ) -> Vec { + apply_skill_filter(skills, tier, false, false, minimal, reduced) + } + // ----------------------------------------------------------------------- // minimal_skills = true // ----------------------------------------------------------------------- @@ -122,7 +200,7 @@ mod tests { "variables", "design-md", ]); - let out = apply_skill_filter(input, ModelTier::Full, true, false); + let out = filter(input, ModelTier::Full, true, false); assert_eq!(names(&out), vec!["schema", "jsonl-format"]); } @@ -132,18 +210,101 @@ mod tests { // keeps exactly `schema` + `jsonl-format` — `jsonl-format-simplified` // is NOT in the allow-set, so it is dropped. let input = skills(&["schema", "jsonl-format-simplified", "layout", "elements"]); - let out = apply_skill_filter(input, ModelTier::Basic, true, false); + let out = filter(input, ModelTier::Basic, true, false); assert_eq!(names(&out), vec!["schema"]); } #[test] fn minimal_skills_true_overrides_reduced_complexity() { - // minimal_skills takes precedence + // minimal_skills takes precedence (early return, reduced ignored). let input = skills(&["schema", "jsonl-format", "layout", "mobile-app"]); - let out = apply_skill_filter(input, ModelTier::Basic, true, true); + let out = filter(input, ModelTier::Basic, true, true); assert_eq!(names(&out), vec!["schema", "jsonl-format"]); } + // ----------------------------------------------------------------------- + // base filter (all tiers) + jsonl dedup + // ----------------------------------------------------------------------- + + #[test] + fn base_filter_drops_mobile_app_on_non_mobile() { + let input = skills(&["schema", "layout", "mobile-app"]); + // Full tier, non-mobile → mobile-app dropped, rest kept. + let out = apply_skill_filter(input, ModelTier::Full, false, false, false, false); + assert_eq!(names(&out), vec!["schema", "layout"]); + } + + #[test] + fn base_filter_drops_landing_copy_antislop_on_mobile() { + let input = skills(&[ + "schema", + "landing-page", + "copywriting", + "anti-slop", + "mobile-app", + ]); + // Full tier, mobile → landing-page/copywriting/anti-slop dropped, + // mobile-app kept (it's a mobile screen). + let out = apply_skill_filter(input, ModelTier::Full, true, false, false, false); + assert_eq!(names(&out), vec!["schema", "mobile-app"]); + } + + #[test] + fn base_filter_drops_design_system_when_explicit_style_guide() { + let input = skills(&["schema", "design-system", "layout"]); + // has_explicit_style_guide = true → design-system dropped. + let out = apply_skill_filter(input, ModelTier::Full, false, true, false, false); + assert_eq!(names(&out), vec!["schema", "layout"]); + } + + #[test] + fn jsonl_dedup_drops_verbose_when_simplified_present() { + let input = skills(&[ + "schema", + "jsonl-format", + "jsonl-format-simplified", + "layout", + ]); + let out = filter(input, ModelTier::Full, false, false); + assert!( + !names(&out).contains(&"jsonl-format"), + "verbose jsonl-format dropped when simplified present" + ); + assert!(names(&out).contains(&"jsonl-format-simplified")); + } + + // ----------------------------------------------------------------------- + // Basic-tier allow-set (non-reduced) + // ----------------------------------------------------------------------- + + #[test] + fn basic_tier_allow_set_drops_non_allowed_skills() { + // `component-composition` / `examples` are not in the Basic allow-set. + let input = skills(&[ + "schema", + "layout", + "component-composition", + "examples", + "jsonl-format-simplified", + ]); + let out = filter(input, ModelTier::Basic, false, false); + let got = names(&out); + assert!(!got.contains(&"component-composition")); + assert!(!got.contains(&"examples")); + assert!(got.contains(&"schema")); + assert!(got.contains(&"layout")); + assert!(got.contains(&"jsonl-format-simplified")); + } + + #[test] + fn standard_tier_keeps_non_allowed_skills() { + // The allow-set is Basic-only; Standard/Full keep everything the base + // filter left. + let input = skills(&["schema", "component-composition", "examples"]); + let out = filter(input.clone(), ModelTier::Standard, false, false); + assert_eq!(out, input, "Standard tier: no Basic allow-set narrowing"); + } + // ----------------------------------------------------------------------- // reduced_complexity + Basic tier -> retryAllowed 8-set // ----------------------------------------------------------------------- @@ -164,7 +325,8 @@ mod tests { "overflow", // MUST be dropped (not in retryAllowed) "icon-catalog", // MUST be dropped ]); - let out = apply_skill_filter(input, ModelTier::Basic, false, true); + // is_mobile = true so `mobile-app` survives the base filter. + let out = apply_skill_filter(input, ModelTier::Basic, true, false, false, true); let got = names(&out); // elements must NOT be present assert!( @@ -198,7 +360,7 @@ mod tests { #[test] fn reduced_complexity_basic_drops_elements() { let input = skills(&["schema", "jsonl-format-simplified", "elements", "layout"]); - let out = apply_skill_filter(input, ModelTier::Basic, false, true); + let out = filter(input, ModelTier::Basic, false, true); let got = names(&out); assert!(!got.contains(&"elements")); assert!(got.contains(&"schema")); @@ -207,13 +369,13 @@ mod tests { } // ----------------------------------------------------------------------- - // reduced_complexity + Standard/Full -> no-op + // reduced_complexity + Standard/Full -> no Basic narrowing // ----------------------------------------------------------------------- #[test] fn reduced_complexity_standard_is_noop() { let input = skills(&["schema", "layout", "elements", "overflow", "anti-slop"]); - let out = apply_skill_filter(input.clone(), ModelTier::Standard, false, true); + let out = filter(input.clone(), ModelTier::Standard, false, true); assert_eq!( out, input, "Standard tier: reduced_complexity must be no-op" @@ -223,18 +385,20 @@ mod tests { #[test] fn reduced_complexity_full_is_noop() { let input = skills(&["schema", "layout", "elements", "overflow"]); - let out = apply_skill_filter(input.clone(), ModelTier::Full, false, true); + let out = filter(input.clone(), ModelTier::Full, false, true); assert_eq!(out, input, "Full tier: reduced_complexity must be no-op"); } // ----------------------------------------------------------------------- - // no filtering -> passthrough + // no filtering -> passthrough (Basic allow-set keeps all-allowed input) // ----------------------------------------------------------------------- #[test] - fn no_filtering_returns_all_skills() { + fn no_filtering_returns_all_allowed_skills() { + // All four are in the Basic allow-set, so the Basic intersection is a + // no-op and the list passes through unchanged. let input = skills(&["schema", "layout", "text-rules", "elements"]); - let out = apply_skill_filter(input.clone(), ModelTier::Basic, false, false); + let out = filter(input.clone(), ModelTier::Basic, false, false); assert_eq!(out, input); } } diff --git a/crates/op-orchestrator/src/prompt.rs b/crates/op-orchestrator/src/prompt.rs index bf8539547..86db1e160 100644 --- a/crates/op-orchestrator/src/prompt.rs +++ b/crates/op-orchestrator/src/prompt.rs @@ -12,8 +12,9 @@ use crate::compact_prompt::build_compact_planning_prompt; use crate::compact_skills::apply_skill_filter; +use crate::design_md_policy::build_design_md_style_policy; use crate::design_type::{detect_design_type, DesignType}; -use crate::model_profile::resolve_model_profile; +use crate::model_profile::{resolve_model_profile, ModelTier}; use crate::plan::{OrchestratorPlan, Subtask}; use crate::style_guide_context::build_planning_style_guide_context; use crate::timeouts::{ @@ -159,14 +160,29 @@ pub fn build_orchestrator_prompt( } } -/// 把解析出的 skill 列表返回(供下游过滤)。 -fn resolve_generation_skills(message: &str) -> Vec { - let ctx = op_ai_skills::resolve_skills( - op_ai_skills::Phase::Generation, - message, - &op_ai_skills::ResolveOptions::default(), - ); - ctx.skills +/// 解析 generation 阶段 skill 列表,带 flag/dynamic/budget 选项(供下游过滤)。 +fn resolve_generation_skills( + message: &str, + opts: &op_ai_skills::ResolveOptions, +) -> Vec { + op_ai_skills::resolve_skills(op_ai_skills::Phase::Generation, message, opts).skills +} + +/// 该 plan 是否代表一整屏移动端页面。 +/// +/// Port of `computeIsMobileFullScreen` (orchestrator-plan-classify.ts:41-58): +/// 窄(≤480)且高(≥480)即整屏;窄而高度为 0/auto 时用 subtask 数 ≥2 +/// 区分"整屏多区块页面"与单卡片 Type 0 组件。TS 的 WeakMap memo 是为了 +/// 跨 status-bar strip 保持一致——Rust 在 strip 之后的最终 plan 上直接算, +/// 无需 memo。 +fn is_mobile_full_screen(plan: &OrchestratorPlan) -> bool { + if plan.root_frame.width > 480.0 { + return false; + } + if plan.root_frame.height >= 480.0 { + return true; + } + plan.subtasks.len() >= 2 } /// 单个 sub-agent 的 LLM 调用输入。 @@ -191,8 +207,72 @@ pub fn build_subagent_prompt( // Resolve the full generation skill set, then apply tier-gated filtering. let model_id = req.model.as_deref().unwrap_or(""); let tier = resolve_model_profile(model_id).tier; - let resolved = resolve_generation_skills(&subtask.label); - let filtered = apply_skill_filter(resolved, tier, minimal_skills, reduced_complexity); + + // design.md payload for the `{{designMdContent}}` template. If the + // structured policy summary is empty (a bare-minimum design.md with only + // free-form text), fall back to the raw markdown so the sub-agent still + // sees the spec. Port of orchestrator-sub-agent.ts:379-384. + let design_md_content = req + .design_md + .as_ref() + .map(|spec| { + let structured = build_design_md_style_policy(spec); + let structured = structured.trim(); + if structured.is_empty() { + spec.raw.trim().to_string() + } else { + structured.to_string() + } + }) + .unwrap_or_default(); + let has_design_md = !design_md_content.is_empty(); + // Rust `OrchestratorPlan` carries only the style-guide NAME (the TS + // `selectedStyleGuideContent` content field has no Rust equivalent yet), + // so `style_guide_name.is_some()` is the faithful proxy for "a guide was + // selected". Port of the flag block in orchestrator-sub-agent.ts:396-416. + let has_explicit_style_guide = plan.style_guide_name.is_some() || req.design_md.is_some(); + let no_style_guide_match = plan.style_guide_name.is_none() && !has_design_md; + + let mut flags = HashMap::new(); + flags.insert("isBasicTier".to_string(), tier == ModelTier::Basic); + flags.insert("hasDesignMd".to_string(), has_design_md); + // No existing-document variable context is wired into `DesignRequest` + // (TS sources this from `request.context.variables`), so this is always + // false on the Rust path today. + flags.insert("hasVariables".to_string(), false); + flags.insert("noStyleGuideMatch".to_string(), no_style_guide_match); + // Element-tools (N-tool) path is not ported to Rust (feature-flag off in + // TS production); `elements`/`elements-cookbook` therefore stay gated off. + flags.insert("hasMcpTools".to_string(), false); + + let mut dynamic_content = HashMap::new(); + if has_design_md { + dynamic_content.insert("designMdContent".to_string(), design_md_content); + } + + // Tier-scaled budget override (orchestrator-sub-agent.ts:414-415). + let budget_override = match tier { + ModelTier::Basic => Some(5200), + ModelTier::Standard => Some(6500), + ModelTier::Full => None, + }; + + let opts = op_ai_skills::ResolveOptions { + flags, + dynamic_content, + budget_override, + ..Default::default() + }; + let resolved = resolve_generation_skills(&subtask.label, &opts); + let is_mobile_screen = is_mobile_full_screen(plan); + let filtered = apply_skill_filter( + resolved, + tier, + is_mobile_screen, + has_explicit_style_guide, + minimal_skills, + reduced_complexity, + ); let mut system_prompt = filtered .iter() diff --git a/crates/op-orchestrator/src/prompt_tests.rs b/crates/op-orchestrator/src/prompt_tests.rs index fca59b2e8..1798de62e 100644 --- a/crates/op-orchestrator/src/prompt_tests.rs +++ b/crates/op-orchestrator/src/prompt_tests.rs @@ -256,6 +256,59 @@ fn subagent_prompt_reduced_complexity_full_tier_is_noop() { ); } +/// Regression guard for the flag-passing fix: a Basic-tier model must load +/// `jsonl-format-simplified` (gated by the `isBasicTier` flag) and drop the +/// verbose `jsonl-format`, while a Full-tier model keeps the verbose one. +/// Before the fix, `resolve_generation_skills` passed empty flags, so the +/// simplified skill could NEVER load for the weak models it targets. +#[test] +fn subagent_prompt_basic_tier_swaps_in_simplified_format_skill() { + // Verbose-only marker (lives solely in jsonl-format.md) and simplified-only + // marker (the parenthesized rectangle arg list lives solely in + // jsonl-format-simplified.md). + const VERBOSE_ONLY: &str = "imageSearchQuery MUST be UNIQUE"; + const SIMPLIFIED_ONLY: &str = "rectangle (width,height,cornerRadius,fill)"; + + let basic_req = DesignRequest { + prompt: "a page".into(), + model: Some("claude-haiku".into()), // Basic tier + provider: None, + design_md: None, + concurrency: 1, + append_context: None, + validation_enabled: true, + visual_ref_enabled: false, + }; + let basic_cr = build_subagent_prompt( + &subtask(), + &plan(), + &basic_req, + AbortFlag::new(), + false, + false, + ); + // req() is model "claude" → Full tier. + let full_cr = + build_subagent_prompt(&subtask(), &plan(), &req(), AbortFlag::new(), false, false); + + assert!( + basic_cr.system_prompt.contains(SIMPLIFIED_ONLY), + "Basic tier must load jsonl-format-simplified" + ); + assert!( + !basic_cr.system_prompt.contains(VERBOSE_ONLY), + "Basic tier must NOT carry the verbose jsonl-format (deduped by simplified)" + ); + assert!( + full_cr.system_prompt.contains(VERBOSE_ONLY), + "Full tier must keep the verbose jsonl-format" + ); + assert!( + !full_cr.system_prompt.contains(SIMPLIFIED_ONLY), + "Full tier must NOT load jsonl-format-simplified (isBasicTier is false)" + ); +} + // ── C4: timeout wiring tests ────────────────────────────────────────────── /// Rich/Minimal mode sets profile-derived timeouts (not None).