SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(agent): auto-fix ring concentricity and stop degrading on quality rejections

Browse source 
A concentricity rejection used to discard a full-skills design
outright, and the retry ladder then dropped skill rungs for a failure
that had nothing to do with capacity — converting one strict check
into a minimal-tier final output. Ring stacks now get a tier-2
force-center repair (centring ignores padding — jian absolute
positioning never adds it — and only structurally hopeless stacks
still reject), and the ladder distinguishes quality rejections from
transport failures: a self-check rejection retries at the SAME skill
tier with the rejection reason injected as feedback, while transport
errors keep the existing degradation path. Remaining fatal checks are
classified for follow-up.
This commit is contained in:
Fini 2026-07-18 16:34:24 +08:00
parent e9cde93377
commit e5e41fbefe
22 changed files with 882 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
crates/op-orchestrator/src/append.rs
View file

@ -103,6 +103,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
})
.collect(),
style_guide_name: None,

33
crates/op-orchestrator/src/concurrent.rs
View file

@ -55,7 +55,7 @@
use crate::model_profile::ModelTier;
use crate::plan::{OrchestratorPlan, Subtask};
use crate::retry::is_non_retryable;
use crate::retry::{is_non_retryable, is_self_check_rejection};
use crate::screen_groups::ScreenGroup;
use crate::subagent::{apply_command_with_reveal, reveal_now_millis, run_subtask_with_reveal_at};
use crate::types::{AbortFlag, DesignRequest, DocSink, LlmClient, Progress, SubtaskOutcome};
@ -221,7 +221,32 @@ pub(crate) async fn run_subtask_retry_ladder(
o.error.is_some() && o.node_count == 0 && !abort.is_set() && !non_retryable
};
// Attempt 2 — reduced_complexity iff Basic tier.
// A self-check quality rejection (`orchestration_self_check` fatally
// rejected otherwise-real, otherwise-parsed content) is not evidence the
// model needs a narrower skill set — the content was fine except for the
// one flagged issue, so throwing skills away on attempt 2 only makes the
// REST of the design worse while doing nothing to fix that issue. Skill
// downgrade stays reserved for attempt 1 failures that actually suggest
// the model is struggling with the full prompt (stream errors, parse
// failures, blank output). Instead, attempt 2 retries at the SAME
// complexity/skill tier with the rejection reason echoed into the
// prompt (`prompt.rs`'s `retry_feedback` block) so the model can fix
// exactly that issue.
let attempt1_self_check_rejection = outcome1
.error
.as_deref()
.is_some_and(is_self_check_rejection);
let attempt2_subtask = if attempt1_self_check_rejection {
Subtask {
retry_feedback: outcome1.error.clone(),
..subtask.clone()
}
} else {
subtask.clone()
};
// Attempt 2 — reduced_complexity iff Basic tier AND attempt 1 wasn't a
// self-check quality rejection.
let outcome2 = if retryable(&outcome1) {
tracing::warn!(
subtask = %subtask.id,
@ -238,13 +263,13 @@ pub(crate) async fn run_subtask_retry_ladder(
});
Some(
run_subtask_with_reveal_at(
subtask,
&attempt2_subtask,
plan,
request,
llm,
sink,
abort,
tier == ModelTier::Basic,
tier == ModelTier::Basic && !attempt1_self_check_rejection,
false,
agent_indicator_epoch,
reveal_now_millis(),

1
crates/op-orchestrator/src/dashboard_columns_tests.rs
View file

@ -36,6 +36,7 @@ fn subtask(id: &str, label: &str, elements: Option<&str>) -> Subtask {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

15
crates/op-orchestrator/src/plan.rs
View file

@ -81,6 +81,15 @@ pub struct Subtask {
/// port of TS `SubTask.existingSectionLabels` (`ai-types.ts:134`)。
#[serde(default, skip_serializing_if = "Option::is_none")]
pub existing_section_labels: Option<Vec<String>>,
/// Set by the retry ladder (`concurrent::run_subtask_retry_ladder`) on
/// attempt 2 ONLY when attempt 1 failed a self-check quality rejection
/// (not a transport/parse failure) — the rejection message, echoed back
/// into the retry prompt so the model can fix exactly that issue while
/// keeping the same skill tier. Execution-only, like
/// `generated_root_id`: never persisted, never present on a freshly
/// planned subtask.
#[serde(skip)]
pub retry_feedback: Option<String>,
}
/// 规划阶段的完整产物。字段对齐规划语料 `decomposition.md`。
@ -200,6 +209,7 @@ pub fn build_fallback_plan(req: &DesignRequest) -> OrchestratorPlan {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
},
Subtask {
id: "main-content".into(),
@ -219,6 +229,7 @@ pub fn build_fallback_plan(req: &DesignRequest) -> OrchestratorPlan {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
},
],
style_guide_name: None,
@ -248,6 +259,7 @@ pub fn build_fallback_plan(req: &DesignRequest) -> OrchestratorPlan {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
})
.collect();
@ -411,6 +423,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
assert!(st.existing_section_labels.is_none());
}
@ -431,6 +444,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: Some(vec!["Hero".into(), "About".into()]),
retry_feedback: None,
};
let labels = st.existing_section_labels.as_ref().unwrap();
assert_eq!(labels[0], "Hero");
@ -453,6 +467,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let json = serde_json::to_string(&st).expect("serialize");
assert!(

7
crates/op-orchestrator/src/plan_normalize.rs
View file

@ -125,6 +125,7 @@ fn ensure_requested_bottom_nav_subtask(plan: &mut OrchestratorPlan, req: &Design
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
});
}
@ -270,6 +271,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}
@ -556,6 +558,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
// chart subtask — LLM-provided height 300, within [inferred*0.6, inferred*1.6]
// inferred for "chart" = 320 → min=192, max=512 → 300 in range → keep 300
@ -572,6 +575,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
// metric subtask — LLM-provided height 0 (invalid) → use inferred = 160
let st_metric = Subtask {
@ -587,6 +591,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
OrchestratorPlan {
root_frame: RootFrameSpec {
@ -687,6 +692,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}],
style_guide_name: None,
};
@ -727,6 +733,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}],
style_guide_name: None,
};

1
crates/op-orchestrator/src/plan_normalize_screen_groups_tests.rs
View file

@ -36,6 +36,7 @@ fn subtask_with_screen(id: &str, label: &str, screen: Option<&str>) -> Subtask {
screen: screen.map(str::to_string),
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

2
crates/op-orchestrator/src/plan_repair.rs
View file

@ -360,6 +360,7 @@ fn coerce_subtask(
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
});
}
@ -412,6 +413,7 @@ fn coerce_subtask(
screen,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
})
}

16
crates/op-orchestrator/src/prompt.rs
View file

@ -1096,6 +1096,22 @@ CRITICAL LAYOUT CONSTRAINTS:\n\
// (Mobile UI guardrails now load from the `mobile-ui` skill — see the
// `isMobileScreen` flag + dynamic-content setup above.)
// Self-check quality-rejection feedback (retry ladder, attempt 2 only —
// see `retry::is_self_check_rejection` + `concurrent::run_subtask_retry_ladder`).
// Echoed back instead of silently narrowing the skill set: the content
// was otherwise real, so the model just needs to fix the one flagged
// geometry/structure issue, at the SAME skill tier attempt 1 used.
if let Some(reason) = subtask.retry_feedback.as_ref() {
user_prompt.push_str(&format!(
"\n\nSELF-CHECK FIX REQUIRED: your previous attempt at this exact \
section was rejected before insertion for this reason: {reason}\n\
- Regenerate the section addressing that reason specifically — do not change \
anything else about the approach.\n\
- Keep using the full skill set and design detail from your previous attempt; \
the rejection was a geometry/structure issue, not a signal to simplify."
));
}
// Port of orchestrator-sub-agent.ts:739-748 — APPEND MODE prompt injection.
if let Some(labels) = subtask.existing_section_labels.as_ref() {
if !labels.is_empty() {

1
crates/op-orchestrator/src/prompt_resolved_style_tests.rs
View file

@ -46,6 +46,7 @@ fn subtask() -> Subtask {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

88
crates/op-orchestrator/src/prompt_tests.rs
View file

@ -132,6 +132,7 @@ fn subagent_prompt_carries_subtask_and_script_format() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(cr.user_prompt.contains("Hero"));
@ -160,6 +161,7 @@ fn subagent_prompt_reduced_complexity_carries_script_format() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), true, false);
assert!(cr.user_prompt.contains("Hero"));
@ -190,6 +192,7 @@ fn subagent_prompt_carries_ts_layout_contract() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
},
Subtask {
id: "categories".into(),
@ -204,6 +207,7 @@ fn subagent_prompt_carries_ts_layout_contract() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
},
];
let (cr, _) = bsp(
@ -249,6 +253,7 @@ fn subagent_prompt_minimal_skills_has_schema_and_script_format() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
// minimal_skills=true: the system prompt should contain schema skill
// content plus the script-gen protocol suffix, but NOT layout/text-rules
@ -287,6 +292,7 @@ fn subagent_prompt_reduced_complexity_basic_is_shorter_than_full() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
// req() uses model "claude" which is Full tier — no narrowing.
// Use a basic-tier model to test narrowing.
@ -327,6 +333,7 @@ fn subagent_prompt_reduced_complexity_full_tier_skill_filtering_is_noop() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
// req() uses "claude" which maps to Full tier → reduced_complexity's skill
// narrowing is a no-op there (unlike Basic, which drops to the
@ -375,6 +382,7 @@ fn subagent_prompt_reduced_complexity_keeps_script_gen_even_on_full_tier() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (full_cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
let (reduced_cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), true, false);
@ -493,6 +501,7 @@ fn subagent_prompt_basic_mobile_food_keeps_mobile_app_skill() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (_, report) = bsp(
@ -546,6 +555,7 @@ fn subagent_prompt_honors_explicit_radius_and_spacing_numbers() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(
@ -606,6 +616,7 @@ fn mobile_food_prompt_avoids_fixed_food_template() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(
@ -683,6 +694,7 @@ fn chinese_mobile_food_prompt_carries_language_consistency_rule() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, report) = bsp(
@ -865,6 +877,7 @@ fn subtask() -> crate::plan::Subtask {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}
@ -987,6 +1000,7 @@ fn subagent_prompt_append_mode_injected_when_labels_present() {
screen: None,
generated_root_id: None,
existing_section_labels: Some(vec!["Hero".into(), "Pricing".into()]),
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(
@ -1020,6 +1034,7 @@ fn subagent_prompt_no_append_mode_when_labels_none() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(
@ -1045,6 +1060,7 @@ fn subagent_prompt_no_append_mode_when_labels_empty() {
screen: None,
generated_root_id: None,
existing_section_labels: Some(vec![]),
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(
@ -1053,6 +1069,74 @@ fn subagent_prompt_no_append_mode_when_labels_empty() {
);
}
// ── retry_feedback: self-check rejection echoed into the retry prompt ─────
/// When the retry ladder sets `retry_feedback` (attempt 2 after a self-check
/// quality rejection — see `concurrent::run_subtask_retry_ladder`), the
/// user prompt must carry the rejection reason and tell the model to keep
/// its full skill set, not simplify.
#[test]
fn subagent_prompt_injects_self_check_feedback_when_present() {
let st = crate::plan::Subtask {
id: "hero".into(),
label: "Hero".into(),
region: crate::plan::Region {
width: 1200.0,
height: 400.0,
},
id_prefix: "hero".into(),
parent_frame_id: None,
elements: None,
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: Some(
"self-check failed: radial-stack-not-concentric at n14: progress-ring track, \
progress arc, and measurable centre content must share one point"
.into(),
),
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(
cr.user_prompt.contains("SELF-CHECK FIX REQUIRED"),
"user_prompt must contain the self-check feedback block"
);
assert!(
cr.user_prompt.contains("radial-stack-not-concentric"),
"user_prompt must echo the actual rejection reason verbatim"
);
assert!(
cr.user_prompt.contains("Keep using the full skill set"),
"user_prompt must tell the model NOT to simplify in response to the rejection"
);
}
/// When `retry_feedback` is `None` (attempt 1, or any non-quality-rejection
/// retry), the user prompt must NOT contain the self-check feedback block.
#[test]
fn subagent_prompt_omits_self_check_feedback_when_absent() {
let st = crate::plan::Subtask {
id: "hero".into(),
label: "Hero".into(),
region: crate::plan::Region {
width: 1200.0,
height: 400.0,
},
id_prefix: "hero".into(),
parent_frame_id: None,
elements: None,
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false);
assert!(
!cr.user_prompt.contains("SELF-CHECK FIX REQUIRED"),
"user_prompt must not mention self-check feedback when there is none"
);
}
// ── B0: subtask_intent ────────────────────────────────────────────────────
/// subtask_intent must include the original request prompt, the subtask label,
@ -1082,6 +1166,7 @@ fn subtask_intent_includes_prompt_label_and_hints() {
screen: Some("home".into()),
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let intent = subtask_intent(&req, &sub);
assert!(
@ -1384,6 +1469,7 @@ fn basic_tier_components_prompt_keeps_both_manifest_and_teaching() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let lib = library_with(5);
@ -1499,6 +1585,7 @@ fn tight_budget_dashboard_keeps_component_composition() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let lib = library_with(5);
@ -1599,6 +1686,7 @@ fn tight_budget_dashboard_without_library_does_not_pin_component_composition() {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
};
let (cr, report) = build_subagent_prompt(

173
crates/op-orchestrator/src/radial_preinsert_tests.rs
View file

@ -217,6 +217,65 @@ fn auto_fix_overlays_safe_nested_authored_ring_before_insert() {
);
}
#[test]
fn non_square_wrapper_and_padded_wrapper_are_now_centred_instead_of_retried() {
// Concentricity is a pure geometric contract: a wrapper's aspect ratio
// and its padding value used to make the strict tier-1 patch decline
// outright (it also resizes/reorders, so it wants a confident square
// ring before touching anything) — but neither actually prevents
// computing a correct centred position, so the lenient tier-2 pass
// (radial_repair_force_center) must now salvage both instead of
// leaving the subtask to retry.
// (wrapper mutation, expected track x/y, expected progress x/y) — track
// is the 120x120 arc, progress the 116x116 arc (`fixed_ring`), so a
// correct centred fix does NOT put them at the same point; it puts
// each at the centre of its own size against the wrapper's full box.
type Point = (f64, f64);
type Case = (fn(&mut Value), Point, Point);
let cases: [Case; 2] = [
(|ring| ring["width"] = json!(240), (60.0, 0.0), (62.0, 2.0)),
(|ring| ring["padding"] = json!(8), (0.0, 0.0), (2.0, 2.0)),
];
for (index, (mutate, expected_track, expected_progress)) in cases.into_iter().enumerate() {
let mut ring = fixed_ring(Some("horizontal"));
mutate(&mut ring);
let mut nodes: Vec<PenNode> = serde_json::from_value(json!([ring])).expect("valid forest");
let before = check_generated_nodes(&nodes, 390.0);
assert!(
has_radial_issue(&before),
"case {index} precondition: {before:?}"
);
assert!(
auto_fix_fixable_issues(&mut nodes, 390.0),
"case {index} must be salvaged, not retried"
);
let repaired = serde_json::to_value(&nodes).expect("serialize repaired forest");
let ring = find_id(&repaired, "ring").expect("ring");
assert_eq!(ring["layout"], json!("none"), "case {index}");
let track = find_id(ring, "track").expect("track");
let progress = find_id(ring, "progress").expect("progress");
assert_eq!(
(track["x"].as_f64(), track["y"].as_f64()),
(Some(expected_track.0), Some(expected_track.1)),
"case {index}: track"
);
assert_eq!(
(progress["x"].as_f64(), progress["y"].as_f64()),
(Some(expected_progress.0), Some(expected_progress.1)),
"case {index}: progress"
);
let after = check_generated_nodes(&nodes, 390.0);
assert!(
!after.has_fatal(),
"case {index} must pass self-check after the lenient fix: {after:?}"
);
}
}
#[test]
fn explicit_but_unrepairable_radial_shapes_are_rejected_without_guessing() {
let cases = [
@ -225,11 +284,6 @@ fn explicit_but_unrepairable_radial_shapes_are_rejected_without_guessing() {
ring["width"] = json!("fill_container");
ring
},
{
let mut ring = fixed_ring(Some("horizontal"));
ring["width"] = json!(240);
ring
},
{
let mut ring = fixed_ring(Some("horizontal"));
ring["children"][1]["width"] = json!(60);
@ -270,11 +324,6 @@ fn high_confidence_but_unsafe_stacks_remain_fatal_for_retry() {
});
ring
},
{
let mut ring = fixed_ring(Some("horizontal"));
ring["padding"] = json!(8);
ring
},
{
let mut ring = fixed_ring(Some("horizontal"));
ring["children"][2]["width"] = json!(160);
@ -439,3 +488,107 @@ fn pure_repair_walks_object_subtrees_without_editor_state() {
);
assert_eq!(root["layout"], json!("vertical"));
}
/// One "Progress Ring" wrapper reconstructed verbatim from the real
/// `savings-goals` subtask script recovered from `0717-6-cc.log` (attempt
/// 2's full script text — attempt 1's own raw nodes weren't logged, only
/// the fatal issue codes for 3 rings at n14/n27/n40, and attempt 2 never
/// reached self-check because of an unrelated JS syntax error). Same
/// `ringSize`, `innerRadius`, `startAngle`, and `Ring Center` box the model
/// actually authored for each goal card. The one addition — `padding: 4`
/// on the ring wrapper — is the plausible source of attempt 1's rejection:
/// this exact structure without it already round-trips through the
/// existing strict tier-1 patch (see
/// `auto_fix_overlays_safe_nested_authored_ring_before_insert` above), so
/// something about attempt 1's copy must have tripped one of tier 1's
/// confidence gates for the fix to have failed twice in a row; padding is
/// the most representative example both because this same script pads
/// every card (`padding: 20`) and because it's exactly the gate this task
/// converts to tier 2.
fn savings_goal_ring(suffix: &str, pct: u32) -> Value {
json!({
"type": "frame",
"id": format!("ring-{suffix}"),
"name": "Progress Ring",
"width": 56,
"height": 56,
"layout": "none",
"padding": 4,
"children": [
{"type": "ellipse", "id": format!("track-{suffix}"), "name": "Ring Track",
"width": 56, "height": 56, "innerRadius": 0.82,
"fill": [{"type": "solid", "color": "#F1F5F9"}]},
{"type": "ellipse", "id": format!("progress-{suffix}"), "name": "Ring Progress",
"width": 56, "height": 56, "innerRadius": 0.82, "startAngle": -90,
"sweepAngle": (360 * pct / 100),
"fill": [{"type": "solid", "color": "#0D9488"}]},
{"type": "frame", "id": format!("centre-{suffix}"), "name": "Ring Center",
"width": 40, "height": 20, "layout": "horizontal",
"alignItems": "center", "justifyContent": "center",
"children": [
{"type": "text", "id": format!("pct-{suffix}"),
"content": format!("{pct}%"), "width": "fit_content", "textGrowth": "auto"}
]}
]
})
}
#[test]
fn savings_goal_rings_from_the_0717_6_cc_incident_are_salvaged_by_auto_fix() {
let rail = json!({
"type": "frame",
"id": "goals-rail",
"name": "Savings Goals Rail",
"width": "fill_container",
"height": "fit_content",
"layout": "horizontal",
"gap": 12,
"children": [
savings_goal_ring("emergency-fund", 72),
savings_goal_ring("new-car", 45),
savings_goal_ring("vacation", 88),
]
});
let mut nodes: Vec<PenNode> = serde_json::from_value(json!([rail])).expect("valid rail");
let before = check_generated_nodes(&nodes, 390.0);
assert!(
has_radial_issue(&before),
"precondition: must reproduce the fatal rejection: {before:?}"
);
assert!(
auto_fix_fixable_issues(&mut nodes, 390.0),
"the real savings-goals rings must be salvaged on attempt 1, not \
fall through to a minimal-skills salvage rung"
);
let repaired = serde_json::to_value(&nodes).expect("serialize repaired rail");
for suffix in ["emergency-fund", "new-car", "vacation"] {
let ring = find_id(&repaired, &format!("ring-{suffix}")).expect("ring");
let track = find_id(ring, &format!("track-{suffix}")).expect("track");
let progress = find_id(ring, &format!("progress-{suffix}")).expect("progress");
assert_eq!(
(track["x"].as_f64(), track["y"].as_f64()),
(Some(0.0), Some(0.0)),
"ring {suffix}: 56x56 track centred in the 56x56 wrapper sits at the origin"
);
assert_eq!(
(track["x"].as_f64(), track["y"].as_f64()),
(progress["x"].as_f64(), progress["y"].as_f64()),
"ring {suffix}: track and progress are the same size, so they share one point"
);
let centre = find_id(ring, &format!("centre-{suffix}")).expect("centre");
assert_eq!(
(centre["x"].as_f64(), centre["y"].as_f64()),
(Some(8.0), Some(18.0)),
"ring {suffix}: the 40x20 Ring Center the model authored centres at (8, 18)"
);
}
let after = check_generated_nodes(&nodes, 390.0);
assert!(
!after.has_fatal(),
"salvaged rail must pass self-check on the same attempt: {after:?}"
);
}

45
crates/op-orchestrator/src/radial_repair.rs
View file

@ -6,6 +6,9 @@ use serde_json::Value;
use crate::types::DocSink;
#[path = "radial_repair_force_center.rs"]
mod radial_repair_force_center;
#[derive(Clone, Copy)]
struct Rect {
w: f64,
@ -57,28 +60,38 @@ pub(crate) fn is_radial_stack_in_flow(v: &Value) -> bool {
/// True when an authored track/progress pair cannot be shown concentrically
/// on the first reveal. Besides flex-flow stacks this catches `layout:none`
/// wrappers whose child coordinates, fixed geometry, or painter order are
/// incomplete. Those cases must be repaired before insertion or retried; the
/// late cleanup should not be the first moment the ring looks correct.
/// incomplete. Concentricity is a pure geometric contract, so this defers to
/// the strict high-confidence patch when it applies, and otherwise to the
/// lenient tier-2 centring check (`radial_repair_force_center`) — a ring is
/// only genuinely unsafe once *both* tiers agree it can't be resolved from
/// the authored/estimated geometry alone.
pub(crate) fn is_authored_radial_stack_unsafe(v: &Value) -> bool {
if radial_layers(v).is_none() {
return false;
}
let Some(patch) = authored_radial_patch(v) else {
return true;
};
authored_patch_changes(v, &patch)
match authored_radial_patch(v) {
Some(patch) => authored_patch_changes(v, &patch),
None => radial_repair_force_center::is_still_off_center(v),
}
}
/// Normalize high-confidence radial stacks directly in an authored JSON
/// forest. This is intentionally independent of `EditorState`: it runs before
/// Normalize radial stacks directly in an authored JSON forest. This is
/// intentionally independent of `EditorState`: it runs before
/// `InsertSubtree`, while the existing sink-based repair remains the late
/// resolved-layout fallback.
///
/// Only fixed, near-square wrappers with similarly sized authored arcs and
/// fully measurable direct children are changed. A high-confidence candidate
/// with unmeasurable centre content stays untouched so self-check retries it;
/// malformed but explicit track/progress pairs are left untouched so
/// self-check can request a retry rather than guessing wrapper geometry.
/// Two tiers, in order:
/// 1. The strict, high-confidence patch below — near-square wrappers with
/// similarly sized authored arcs and fully measurable direct children —
/// which also fills in missing arc/centre dimensions and fixes painter
/// order.
/// 2. When tier 1 declines (non-square wrapper, out-of-range arc ratio,
/// ambiguous painter order, asymmetric padding, …), the lenient tier-2
/// pass re-centres whatever tier 1 left alone: concentricity is a
/// geometry fact once `radial_layers` already recognised the ring, so
/// there is no confidence gate left to apply. Only a child whose size
/// can be neither read nor estimated at all stays untouched, so
/// self-check keeps reporting it instead of guessing.
pub(crate) fn repair_authored_radial_stacks(value: &mut Value) -> bool {
match value {
Value::Array(nodes) => {
@ -94,8 +107,10 @@ pub(crate) fn repair_authored_radial_stacks(value: &mut Value) -> bool {
}
fn repair_authored_radial_node(node: &mut Value) -> bool {
let patch = authored_radial_patch(node);
let mut changed = patch.is_some_and(|patch| apply_authored_radial_patch(node, patch));
let mut changed = match authored_radial_patch(node) {
Some(patch) => apply_authored_radial_patch(node, patch),
None => radial_repair_force_center::force_concentric_radial_stack(node),
};
if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) {
for child in children {
changed |= repair_authored_radial_node(child);

193
crates/op-orchestrator/src/radial_repair_force_center.rs Normal file
View file

@ -0,0 +1,193 @@
//! Lenient (tier-2) concentric repair for authored radial stacks.
//!
//! `authored_radial_patch` (in the parent module) is the strict,
//! high-confidence pass: besides repositioning, it also *resizes* things
//! (fills in missing arc/centre dimensions) and gates on wrapper aspect
//! ratio, arc-to-parent-size ratio, and zero padding to be confident that's
//! safe. Those gates are about whether the ring is worth reshaping
//! automatically — not about whether it's safe to *centre*.
//!
//! Concentricity and front-to-back paint order carry no such wrapper-shape
//! judgment call. Once `radial_layers` has already recognised a genuine
//! track/progress (or segmented-donut) ring — and `radial_layer_order` has
//! derived an unambiguous centre/progress/track order for it — sharing one
//! centre point and painting centre-then-progress-then-track are geometric
//! and structural facts, not guesses, regardless of the wrapper's aspect
//! ratio. Padding doesn't even enter into it: jian positions an explicit
//! `x`/`y` child of a `layout:"none"` frame from the frame's *border* box
//! (taffy's absolute-layout algorithm only adds `border`, never `padding`,
//! to an inset that's already set) — so the wrapper's `padding` value is
//! irrelevant to where a centred child actually renders, and centring
//! always targets the full `width`×`height` box regardless of it.
//!
//! So this module re-centres and reorders whatever `authored_radial_patch`
//! declined to touch for wrapper-shape or padding reasons, without resizing
//! anything it didn't already have a size for. What still keeps a stack
//! unfixable here (echoed back instead of guessed): more than one
//! centre-content child (ambiguous order), a child whose size can be
//! neither read nor estimated at all, a child that doesn't fit inside the
//! wrapper's box no matter how it's centred (an authoring-size bug, not a
//! position bug), or arc diameters too mismatched to plausibly be the same
//! ring (`radial_layers` classifies track vs. progress by sweep angle /
//! naming alone, not by size, so nothing upstream has already vetted that
//! the pairing is size-plausible).
use serde_json::Value;
use super::{
estimated_subtree_size, has_numeric, is_arc_ellipse, numeric, radial_layer_order,
radial_layers, valid_size, MIN_SAFE_ARC_DIAMETER_RATIO,
};
/// `(x, y, width, height)`.
type Placement = (f64, f64, f64, f64);
/// Re-centre every direct child of `node` on the wrapper's full box and fix
/// its paint order to centre/progress/track. Returns `false` (no-op) when
/// `node` isn't a recognised radial stack with an unambiguous layer order,
/// or when the wrapper's own size or any child's size can't be read or
/// estimated — those cases are left for the caller to report rather than
/// guess at.
pub(super) fn force_concentric_radial_stack(node: &mut Value) -> bool {
let Some(order) = radial_layer_order(node) else {
return false;
};
let Some(placements) = concentric_placements(node) else {
return false;
};
let mut changed = set_if_different(node, "layout", Value::String("none".into()));
changed |= set_if_different(node, "gap", Value::from(0.0));
changed |= set_if_different(node, "justifyContent", Value::String("start".into()));
changed |= set_if_different(node, "alignItems", Value::String("start".into()));
let kids = node
.get_mut("children")
.and_then(Value::as_array_mut)
.expect("radial_layer_order already confirmed children exist");
changed |= order.iter().copied().ne(0..kids.len());
// Pair each original child with its computed placement before
// reordering, so the permutation below carries the right placement
// along with the right child.
let mut slots: Vec<Option<(Value, Placement)>> = std::mem::take(kids)
.into_iter()
.zip(placements)
.map(Some)
.collect();
for index in order {
let Some((mut child, (x, y, width, height))) = slots.get_mut(index).and_then(Option::take)
else {
continue;
};
let had_width = has_numeric(&child, "width");
let had_height = has_numeric(&child, "height");
changed |= set_if_different(&mut child, "x", Value::from(x));
changed |= set_if_different(&mut child, "y", Value::from(y));
if !had_width {
changed |= set_if_different(&mut child, "width", Value::from(width.round()));
}
if !had_height {
changed |= set_if_different(&mut child, "height", Value::from(height.round()));
}
kids.push(child);
}
changed
}
/// True when `node` is a recognised radial stack that this lenient pass
/// still can't call safe: the layer order is ambiguous, the children
/// aren't already in centre/progress/track order, it isn't overlaid
/// (`layout:none`), a child's authored x/y doesn't match the centred
/// placement, or the placements can't even be computed (unmeasurable
/// wrapper/child size). `node` not being a radial stack at all is *not*
/// unsafe — there's nothing to check — so this checks `radial_layers`
/// itself first rather than reusing `radial_layer_order`'s `None`, which
/// also covers the very different "ambiguous order" case.
pub(super) fn is_still_off_center(node: &Value) -> bool {
if radial_layers(node).is_none() {
return false;
}
let Some(order) = radial_layer_order(node) else {
return true;
};
let Some(placements) = concentric_placements(node) else {
return true;
};
if node.get("layout").and_then(Value::as_str) != Some("none") {
return true;
}
if order.iter().copied().ne(0..placements.len()) {
return true;
}
let kids = node
.get("children")
.and_then(Value::as_array)
.map(Vec::as_slice)
.unwrap_or(&[]);
placements
.iter()
.zip(kids)
.any(|((x, y, _width, _height), child)| {
numeric(child, "x") != Some(*x) || numeric(child, "y") != Some(*y)
})
}
/// The centred `(x, y, width, height)` every direct child of `node` should
/// sit at against the wrapper's full `width`×`height` box (padding is
/// deliberately not subtracted — see the module doc: jian positions an
/// explicit-inset absolute child from the border box, ignoring the
/// parent's padding, so a padding-aware centre would be centring against a
/// box the renderer never actually uses). Indexed in the child array's
/// *original* order — callers that also reorder must carry each placement
/// along with its child through the permutation, not re-look-up by new
/// position. `width`/`height` are the authored size when present, else the
/// best estimate used to compute the centred `x`/`y` — callers only need
/// to persist them when the child had none authored. `None` means there is
/// nothing safe to compute: the wrapper's own size or some child's size
/// can't be read or estimated, a child doesn't fit inside the wrapper's
/// box at all (an authoring-size bug translation can't paper over), or the
/// arcs' diameters are too mismatched to plausibly be the same ring.
fn concentric_placements(node: &Value) -> Option<Vec<Placement>> {
let box_w = numeric(node, "width")?;
let box_h = numeric(node, "height")?;
if !valid_size(box_w, box_h) {
return None;
}
let kids = node.get("children").and_then(Value::as_array)?;
let mut placements = Vec::with_capacity(kids.len());
let mut arc_diameters = Vec::new();
for child in kids {
let estimate = estimated_subtree_size(child);
let width = numeric(child, "width").or_else(|| estimate.map(|size| size.0))?;
let height = numeric(child, "height").or_else(|| estimate.map(|size| size.1))?;
if !valid_size(width, height) || width > box_w + 0.5 || height > box_h + 0.5 {
return None;
}
if is_arc_ellipse(child) {
arc_diameters.push(width.max(height));
}
let x = ((box_w - width) / 2.0).round();
let y = ((box_h - height) / 2.0).round();
placements.push((x, y, width, height));
}
let min_arc = arc_diameters.iter().copied().fold(f64::INFINITY, f64::min);
let max_arc = arc_diameters.iter().copied().fold(0.0, f64::max);
if max_arc > 0.0 && min_arc / max_arc < MIN_SAFE_ARC_DIAMETER_RATIO {
return None;
}
Some(placements)
}
fn set_if_different(node: &mut Value, key: &str, value: Value) -> bool {
if node.get(key) == Some(&value) {
return false;
}
node[key] = value;
true
}
#[cfg(test)]
#[path = "radial_repair_force_center_tests.rs"]
mod tests;

154
crates/op-orchestrator/src/radial_repair_force_center_tests.rs Normal file
View file

@ -0,0 +1,154 @@
use super::*;
use serde_json::json;
/// A ring shaped like the wrapper this module targets: an unambiguous
/// track/progress pair plus one centre-content child, all direct children
/// of a `frame`. Children start in authoring order (track, progress,
/// centre) — canonical paint order is centre, progress, track — so tests
/// also exercise the reordering half of the fix.
fn ring(width: f64, height: f64, extra: Option<Value>) -> Value {
let mut node = json!({
"type": "frame",
"id": "ring",
"width": width,
"height": height,
"layout": "vertical",
"children": [
{"type": "ellipse", "id": "track", "width": 56, "height": 56, "innerRadius": 0.82},
{"type": "ellipse", "id": "progress", "width": 56, "height": 56,
"innerRadius": 0.82, "startAngle": -90, "sweepAngle": 230},
{"type": "frame", "id": "centre", "width": 32, "height": 18,
"children": [{"type": "text", "id": "pct", "content": "64%"}]}
]
});
if let Some(extra) = extra {
for (key, value) in extra.as_object().expect("extra must be an object") {
node[key] = value.clone();
}
}
node
}
fn child<'a>(v: &'a Value, id: &str) -> &'a Value {
v["children"]
.as_array()
.expect("children")
.iter()
.find(|c| c["id"] == id)
.unwrap_or_else(|| panic!("missing child {id}"))
}
#[test]
fn centres_a_non_square_wrapper_tier_one_declines_on_aspect_alone() {
let mut node = ring(240.0, 56.0, None);
assert!(is_still_off_center(&node), "precondition: not yet centred");
assert!(force_concentric_radial_stack(&mut node));
assert_eq!(node["layout"], json!("none"));
let (track, progress) = (child(&node, "track"), child(&node, "progress"));
assert_eq!(
(track["x"].as_f64(), track["y"].as_f64()),
(Some(92.0), Some(0.0)),
"56-wide arc centred in a 240-wide box sits at (240-56)/2 = 92"
);
assert_eq!(
(track["x"].as_f64(), track["y"].as_f64()),
(progress["x"].as_f64(), progress["y"].as_f64()),
"track and progress are the same size, so they share one point"
);
assert!(!is_still_off_center(&node), "must be safe after the fix");
}
#[test]
fn ignores_padding_because_jian_positions_absolute_children_from_the_border_box() {
let mut padded = ring(56.0, 56.0, Some(json!({"padding": 8})));
let mut bare = ring(56.0, 56.0, None);
assert!(force_concentric_radial_stack(&mut padded));
assert!(force_concentric_radial_stack(&mut bare));
assert_eq!(
child(&padded, "track")["x"],
child(&bare, "track")["x"],
"padding must not shift the computed centre — jian ignores it for \
layout:none absolute children (only `border` is added to an \
explicit inset, never `padding`)"
);
assert!(!is_still_off_center(&padded));
}
#[test]
fn reorders_children_to_centre_progress_track_paint_order() {
let mut node = ring(56.0, 56.0, None);
let before: Vec<&str> = node["children"]
.as_array()
.unwrap()
.iter()
.map(|c| c["id"].as_str().unwrap())
.collect();
assert_eq!(before, ["track", "progress", "centre"], "precondition");
assert!(force_concentric_radial_stack(&mut node));
let after: Vec<&str> = node["children"]
.as_array()
.unwrap()
.iter()
.map(|c| c["id"].as_str().unwrap())
.collect();
assert_eq!(after, ["centre", "progress", "track"]);
}
#[test]
fn declines_when_a_child_does_not_fit_in_the_wrapper_at_all() {
let mut node = ring(56.0, 56.0, None);
node["children"][2]["width"] = json!(160);
assert!(!force_concentric_radial_stack(&mut node));
assert!(
is_still_off_center(&node),
"an oversized child is an authoring bug, not a position bug"
);
}
#[test]
fn declines_when_arc_diameters_are_too_mismatched_to_be_the_same_ring() {
let mut node = ring(120.0, 120.0, None);
node["children"][0]["width"] = json!(120);
node["children"][0]["height"] = json!(120);
node["children"][1]["width"] = json!(60);
node["children"][1]["height"] = json!(60);
assert!(!force_concentric_radial_stack(&mut node));
assert!(is_still_off_center(&node));
}
#[test]
fn declines_when_more_than_one_direct_child_is_non_arc_centre_content() {
let mut node = ring(56.0, 56.0, None);
node["children"].as_array_mut().unwrap().push(
json!({"type": "text", "id": "extra-label", "width": 20, "height": 10, "content": "x"}),
);
assert!(!force_concentric_radial_stack(&mut node));
assert!(is_still_off_center(&node));
}
#[test]
fn is_a_no_op_on_a_node_that_is_not_a_radial_stack_at_all() {
let mut plain = json!({
"type": "frame",
"id": "row",
"width": 200,
"height": 40,
"layout": "horizontal",
"children": [
{"type": "ellipse", "id": "a", "width": 32, "height": 32},
{"type": "ellipse", "id": "b", "width": 32, "height": 32}
]
});
assert!(!force_concentric_radial_stack(&mut plain));
assert!(!is_still_off_center(&plain));
}

50
crates/op-orchestrator/src/retry.rs
View file

@ -29,6 +29,26 @@ pub(crate) fn is_non_retryable(msg: &str) -> bool {
|| lower.contains("invalid proxy configuration")
}
/// True when a subtask failed because our own post-generation self-check
/// (`orchestration_self_check`) rejected otherwise-parsed, otherwise-real
/// content — a geometry/quality judgment on THIS model's output — rather
/// than a transport, parsing, or capability failure (stream error, script
/// syntax error, blank output, `InsertSubtree` rejection).
///
/// The distinction matters for the retry ladder
/// (`concurrent::run_subtask_retry_ladder`): a quality rejection is not
/// evidence the model needs a narrower skill set to succeed — the content
/// was otherwise fine, so throwing away skills on attempt 2 only makes the
/// *rest* of the design worse while doing nothing to fix the one flagged
/// issue. Skill-tier downgrade should stay reserved for the failures that
/// actually suggest the model is struggling with the full prompt (timeouts,
/// stream errors, safety-scanner stalls) — see `subagent::run_subtask_with_reveal_at`'s
/// `fail(format!("self-check failed: {message}"))` call site for the exact
/// prefix this matches.
pub(crate) fn is_self_check_rejection(msg: &str) -> bool {
msg.starts_with("self-check failed: ")
}
#[cfg(test)]
mod tests {
use super::*;
@ -91,6 +111,36 @@ mod tests {
fn socket_closed_is_retryable() {
assert!(!is_non_retryable("socket closed unexpectedly"));
}
// ── is_self_check_rejection ─────────────────────────────────────────────
#[test]
fn self_check_failure_message_is_a_self_check_rejection() {
assert!(is_self_check_rejection(
"self-check failed: radial-stack-not-concentric at n14: ..."
));
}
#[test]
fn stream_error_is_not_a_self_check_rejection() {
assert!(!is_self_check_rejection(
"stream disconnected before completion"
));
}
#[test]
fn script_parse_error_is_not_a_self_check_rejection() {
assert!(!is_self_check_rejection(
"script error: unexpected end of string"
));
}
#[test]
fn blank_container_is_not_a_self_check_rejection() {
assert!(!is_self_check_rejection(
"blank container root produced no content nodes"
));
}
}
#[cfg(test)]

1
crates/op-orchestrator/src/retry_subtask_tests.rs
View file

@ -55,6 +55,7 @@ fn failed_subtask(parent_frame_id: Option<&str>) -> Subtask {
screen: Some("Home".into()),
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

125
crates/op-orchestrator/src/run_tests.rs
View file

@ -48,6 +48,25 @@ fn req_standard() -> DesignRequest {
}
}
// Basic tier model id — the ONLY tier where attempt 2's `reduced_complexity`
// flag has any effect at all (`compact_skills::apply_skill_filter`'s doc:
// "Basic tier only"), so it's the tier that can actually distinguish this
// module's quality-vs-transport retry-ladder split.
fn req_basic() -> DesignRequest {
DesignRequest {
prompt: "a landing page".into(),
// "glm-4-plus" matches Basic tier in model_profile table
model: Some("glm-4-plus".into()),
provider: None,
design_md: None,
concurrency: 1,
append_context: None,
validation_enabled: true,
visual_ref_enabled: false,
}
}
const PLAN_JSON: &str = r##"{
"rootFrame": { "id": "root", "name": "Page", "width": 1200, "height": 800,
"layout": "vertical", "gap": 0,
@ -80,6 +99,25 @@ fn node_json(prefix: &str) -> String {
)
}
// A radial ring whose progress arc (60px) is far smaller than its track
// (120px) — `orchestration_self_check`'s `radial-stack-not-concentric`
// flags this, and neither repair tier can auto-fix it (the arc-diameter
// mismatch is too implausible to guess a fix for; see
// `radial_preinsert_tests::explicit_but_unrepairable_radial_shapes_are_rejected_without_guessing`).
// This parses fine as script-gen — the rejection comes from self-check, not
// from a parse/stream failure — so it's the fixture for proving the retry
// ladder treats a QUALITY rejection differently from a transport failure.
fn radial_reject_script() -> String {
r##"I(null, {"type":"frame","name":"Ring Section","x":0,"y":0,"width":1200,"height":300,"children":[
{"type":"frame","name":"Steps Ring","width":120,"height":120,"children":[
{"type":"ellipse","name":"track","width":120,"height":120,"innerRadius":0.82,"sweepAngle":360,"fill":[{"type":"solid","color":"#22C55E"}]},
{"type":"ellipse","name":"progress","width":60,"height":60,"innerRadius":0.82,"startAngle":-90,"sweepAngle":264,"fill":[{"type":"solid","color":"#22C55E"}]},
{"type":"frame","name":"centre","width":80,"height":44,"children":[{"type":"text","content":"64%"}]}
]}
]});"##
.into()
}
fn existing_root_json(
id: &str,
name: &str,
@ -415,6 +453,93 @@ fn subtask_retries_on_attempt1_zero_succeeds_on_attempt2() {
assert_eq!(sink.batch_depth, 0);
}
/// A self-check quality rejection on attempt 1 must NOT downgrade attempt
/// 2's skill tier, even on a Basic-tier model that would otherwise always
/// narrow to `retryAllowed` on retry — the content was real, just flagged
/// for one geometry issue, so throwing skills away only makes the rest of
/// the design worse. Attempt 2's prompt must also carry the rejection
/// reason so the model can fix exactly that issue.
#[test]
fn self_check_rejection_keeps_full_skills_and_injects_feedback_on_attempt2() {
let llm = ScriptedLlm::new(vec![
ScriptResponse::Text(PLAN_JSON.into()),
// hero attempt 1 (Basic tier, full complexity): parses fine, but
// self-check fatally rejects the mismatched ring — zero nodes land.
ScriptResponse::Text(radial_reject_script()),
// hero attempt 2: must stay full complexity despite Basic tier.
ScriptResponse::Text(node_json("hero")),
// feat attempt 1: succeeds normally.
ScriptResponse::Text(node_json("feat")),
]);
let mut sink = VecDocSink::new();
let mut on_progress = |_p: Progress| {};
let summary = futures::executor::block_on(Orchestrator::new().run(
req_basic(),
&mut sink,
&llm,
&mut on_progress,
&AbortFlag::new(),
&stub_providers(),
))
.expect("attempt 2 recovers after the self-check rejection");
assert_eq!(summary.subtasks.len(), 2);
// Call order: [0] planning, [1] hero attempt 1, [2] hero attempt 2, [3] feat.
let prompts = llm.system_prompts();
assert_eq!(prompts.len(), 4, "unexpected call count: {prompts:?}");
assert_eq!(
prompts[1], prompts[2],
"attempt 2 after a self-check rejection must resolve the IDENTICAL \
(full) skill set attempt 1 used — a Basic-tier model would \
otherwise narrow this to the retryAllowed set"
);
}
/// The mirror case: an attempt-1 TRANSPORT failure (not a self-check
/// rejection) on a Basic-tier model must still downgrade attempt 2 to
/// `reduced_complexity`, exactly as before this task — skill downgrade
/// stays reserved for failures that suggest the model is struggling with
/// the full prompt, not for a quality gate on otherwise-fine content.
#[test]
fn transport_failure_still_downgrades_skills_on_attempt2_for_basic_tier() {
use crate::types::LlmError;
let llm = ScriptedLlm::new(vec![
ScriptResponse::Text(PLAN_JSON.into()),
// hero attempt 1: a stream error — NOT a self-check rejection.
ScriptResponse::Fail(LlmError {
message: "stream disconnected before completion".into(),
aborted: false,
}),
// hero attempt 2: reduced_complexity narrows the skill set.
ScriptResponse::Text(node_json("hero")),
// feat attempt 1: succeeds normally.
ScriptResponse::Text(node_json("feat")),
]);
let mut sink = VecDocSink::new();
let mut on_progress = |_p: Progress| {};
let summary = futures::executor::block_on(Orchestrator::new().run(
req_basic(),
&mut sink,
&llm,
&mut on_progress,
&AbortFlag::new(),
&stub_providers(),
))
.expect("attempt 2 recovers after the transport failure");
assert_eq!(summary.subtasks.len(), 2);
let prompts = llm.system_prompts();
assert_eq!(prompts.len(), 4, "unexpected call count: {prompts:?}");
assert!(
prompts[2].len() < prompts[1].len(),
"attempt 2 after a plain transport failure must still narrow to the \
reduced-complexity skill set on Basic tier (attempt 1: {} chars, \
attempt 2: {} chars)",
prompts[1].len(),
prompts[2].len()
);
}
/// Subtask fails all 3 attempts → `OrchestratorError::AllFailed` with the
/// final failure context.
#[test]

1
crates/op-orchestrator/src/scaffold_tests.rs
View file

@ -205,6 +205,7 @@ fn st(id: &str, label: &str) -> Subtask {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

1
crates/op-orchestrator/src/screen_groups_tests.rs
View file

@ -21,6 +21,7 @@ fn subtask_with_screen(id: &str, screen: Option<&str>) -> Subtask {
screen: screen.map(|s| s.to_string()),
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

1
crates/op-orchestrator/src/spawn_concurrent.rs
View file

@ -136,6 +136,7 @@ fn plan_from_state(sink: &dyn DocSink, specs: &[SpawnAgentSpec]) -> Orchestrator
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
})
.collect();

1
crates/op-orchestrator/src/subagent.rs
View file

@ -755,6 +755,7 @@ mod tests {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}

1
crates/op-orchestrator/src/subagent_reveal_tests.rs
View file

@ -52,6 +52,7 @@ fn f2_subtask() -> Subtask {
screen: None,
generated_root_id: None,
existing_section_labels: None,
retry_feedback: None,
}
}