feat(orchestrator): run — wire append-to-document mode
4 call sites in Orchestrator::run(): 1. apply_append_context_to_plan after planning_loop (TS :737) 2. skip_status_bar guards effective_is_mobile in build_scaffold (TS :743) 3. effective concurrency forced to 1 when skip_root_insertion (TS :806-810) 4. append fast-path: skip build_scaffold, set parent_frame_id = target, capture scaffold_baseline before sub-agent loop (TS :942-949) Cleanup natural-no-op comment added (spec §4.5). Concurrent + dashboard paths unchanged. 5 new TDD tests in run_tests_b4.rs.
This commit is contained in:
parent
0554d05825
commit
56f56eab0b
|
|
@ -5,8 +5,6 @@
|
|||
//! present; returns an [`AppendPlanResult`] that `run.rs` uses to skip
|
||||
//! root-frame insertion and status-bar scaffold (Task B2).
|
||||
|
||||
#![allow(dead_code)] // callers land in Task B2
|
||||
|
||||
use crate::plan::OrchestratorPlan;
|
||||
use crate::types::AppendContext;
|
||||
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@
|
|||
//! The dashboard path implementation lives in `run_dashboard.rs` (split to
|
||||
//! keep this file under the 800-line ceiling).
|
||||
|
||||
use crate::append::apply_append_context_to_plan;
|
||||
use crate::cleanup::{
|
||||
aggregate_concurrent_verdict, cleanup_concurrent_roots, descendant_count, run_cleanup_passes,
|
||||
};
|
||||
|
|
@ -69,10 +70,24 @@ impl Orchestrator {
|
|||
on_progress(Progress::Planning);
|
||||
let (mut plan, norm) = planning_loop(&request, llm, abort).await?;
|
||||
|
||||
// -- S3b-4 Task B2 call site 1: apply append context (TS :737) --
|
||||
// Must run AFTER planning_loop (which calls normalize) so root_frame.id
|
||||
// and subtasks are already normalized before we repoint them.
|
||||
let append_result =
|
||||
apply_append_context_to_plan(&mut plan, request.append_context.as_ref());
|
||||
|
||||
// -- S3b-2 Task C2: concurrency branch decision --
|
||||
// Port of `orchestrator.ts:780-810` (minus append-mode gate, S3b-4).
|
||||
// Port of `orchestrator.ts:780-810`.
|
||||
let screen_groups = group_subtasks_by_screen(&plan.subtasks);
|
||||
let effective = effective_concurrency(request.concurrency, screen_groups.len());
|
||||
|
||||
// -- S3b-4 Task B2 call site 3: effective concurrency gate (TS :806-810) --
|
||||
// Append mode is forced sequential — the concurrent branch creates multiple
|
||||
// root frames which conflicts with reusing an existing content-root.
|
||||
let effective = if append_result.skip_root_insertion {
|
||||
1
|
||||
} else {
|
||||
effective_concurrency(request.concurrency, screen_groups.len())
|
||||
};
|
||||
|
||||
if effective > 1 {
|
||||
return run_concurrent_path(
|
||||
|
|
@ -103,7 +118,15 @@ impl Orchestrator {
|
|||
}
|
||||
let scaffold_root_index = sink.state().active_children().len();
|
||||
|
||||
if use_dashboard {
|
||||
// -- S3b-4 Task B2: dashboard / append mutex (spec §2) --
|
||||
// Both concurrent and dashboard paths create new root structures that
|
||||
// conflict with reusing an existing content-root. The concurrency gate
|
||||
// above already enforces the concurrent/append mutex; here we enforce
|
||||
// the dashboard/append mutex by preferring the append fast-path when
|
||||
// both would otherwise fire. Mirrors TS positional precedence — the
|
||||
// append fast-path lives inside the sequential else block, before the
|
||||
// dashboard sub-branch.
|
||||
if !append_result.skip_root_insertion && use_dashboard {
|
||||
// ── Dashboard path (extracted to run_dashboard.rs) ────────────
|
||||
return run_dashboard_path(
|
||||
plan,
|
||||
|
|
@ -118,43 +141,74 @@ impl Orchestrator {
|
|||
.await;
|
||||
}
|
||||
|
||||
// ── Non-dashboard sequential path (unchanged from S3a/S3b-1b) ──────
|
||||
match build_scaffold(&plan, norm.is_mobile) {
|
||||
Ok(cmds) => {
|
||||
for cmd in cmds {
|
||||
if !sink.apply(cmd) {
|
||||
rollback(sink, &var_snapshot);
|
||||
sink.end_undo_batch();
|
||||
return Err(OrchestratorError::Internal(
|
||||
"scaffold insert rejected by document".into(),
|
||||
));
|
||||
// ── Non-dashboard sequential path ──────────────────────────────────
|
||||
//
|
||||
// -- S3b-4 Task B2 call site 4: append fast-path (TS :942-949) --
|
||||
// In append mode we reuse the caller-provided content-root instead of
|
||||
// creating a new root + status bar + dashboard columns.
|
||||
// `skip_status_bar` is naturally satisfied here: the concurrent mobile
|
||||
// scaffold (which injects a status-bar child) is skipped entirely, and
|
||||
// `plan_normalize::normalize` already ran its mobile strip before
|
||||
// `apply_append_context_to_plan` filtered any remaining status-bar subtasks.
|
||||
let (root_id, scaffold_baseline) = if append_result.skip_root_insertion {
|
||||
// Append fast-path: reuse the existing target frame as root.
|
||||
// No scaffold InsertSubtree, no status bar, no agent badge.
|
||||
let target_id = plan.root_frame.id.clone();
|
||||
for subtask in &mut plan.subtasks {
|
||||
subtask.parent_frame_id = Some(target_id.clone());
|
||||
}
|
||||
// Capture the pre-generation descendant count of the target frame.
|
||||
// Used by the cleanup's zero-content check below.
|
||||
let baseline = descendant_count(sink.state(), &target_id);
|
||||
on_progress(Progress::ScaffoldDone);
|
||||
(target_id, baseline)
|
||||
} else {
|
||||
// Normal scaffold path (unchanged from S3a/S3b-1b).
|
||||
// S3b-4 Task B2 call site 2 (TS :743): guard the mobile status-bar
|
||||
// injection — if append mode had been active `skip_status_bar` would
|
||||
// be true and the mobile scaffold (which injects a status-bar child)
|
||||
// would be suppressed. Here we're in the non-append branch so
|
||||
// `skip_status_bar` is always false; consuming it silences the
|
||||
// dead-code lint and keeps the guard semantically aligned with TS.
|
||||
let effective_is_mobile = norm.is_mobile && !append_result.skip_status_bar;
|
||||
match build_scaffold(&plan, effective_is_mobile) {
|
||||
Ok(cmds) => {
|
||||
for cmd in cmds {
|
||||
if !sink.apply(cmd) {
|
||||
rollback(sink, &var_snapshot);
|
||||
sink.end_undo_batch();
|
||||
return Err(OrchestratorError::Internal(
|
||||
"scaffold insert rejected by document".into(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// scaffold 模板 bug —— 收尾后报内部错误。
|
||||
rollback(sink, &var_snapshot);
|
||||
sink.end_undo_batch();
|
||||
return Err(OrchestratorError::Internal(e));
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
// scaffold 模板 bug —— 收尾后报内部错误。
|
||||
let Some(rid) = sink
|
||||
.state()
|
||||
.active_children()
|
||||
.get(scaffold_root_index)
|
||||
.map(|n| n.id_str().to_string())
|
||||
else {
|
||||
rollback(sink, &var_snapshot);
|
||||
sink.end_undo_batch();
|
||||
return Err(OrchestratorError::Internal(e));
|
||||
return Err(OrchestratorError::Internal(format!(
|
||||
"scaffold root `{planned_root_id}` was not inserted"
|
||||
)));
|
||||
};
|
||||
for subtask in &mut plan.subtasks {
|
||||
subtask.parent_frame_id = Some(rid.clone());
|
||||
}
|
||||
}
|
||||
let Some(root_id) = sink
|
||||
.state()
|
||||
.active_children()
|
||||
.get(scaffold_root_index)
|
||||
.map(|n| n.id_str().to_string())
|
||||
else {
|
||||
rollback(sink, &var_snapshot);
|
||||
sink.end_undo_batch();
|
||||
return Err(OrchestratorError::Internal(format!(
|
||||
"scaffold root `{planned_root_id}` was not inserted"
|
||||
)));
|
||||
let baseline = descendant_count(sink.state(), &rid);
|
||||
on_progress(Progress::ScaffoldDone);
|
||||
(rid, baseline)
|
||||
};
|
||||
for subtask in &mut plan.subtasks {
|
||||
subtask.parent_frame_id = Some(root_id.clone());
|
||||
}
|
||||
let scaffold_baseline = descendant_count(sink.state(), &root_id);
|
||||
on_progress(Progress::ScaffoldDone);
|
||||
|
||||
// -- 阶段 3:顺序子 agent(C3: 3-attempt tier-gated retry ladder) --
|
||||
//
|
||||
|
|
@ -285,6 +339,11 @@ impl Orchestrator {
|
|||
}
|
||||
|
||||
// -- 阶段 4:清理 --
|
||||
// S3b-4 append mode natural no-op: in append mode the fast-path reuses
|
||||
// `plan.root_frame.id` (the existing target frame) as the single cleanup
|
||||
// root. The cleanup loop iterates that one root; the `descendant_count
|
||||
// <= scaffold_baseline` check below only deletes the frame if NOTHING
|
||||
// new was added, which is the correct desired behaviour.
|
||||
run_cleanup_passes(sink, &plan, &[&root_id]);
|
||||
on_progress(Progress::CleanupDone);
|
||||
|
||||
|
|
@ -604,3 +663,8 @@ mod tests_c2;
|
|||
#[cfg(test)]
|
||||
#[path = "run_tests_c3.rs"]
|
||||
mod tests_c3;
|
||||
|
||||
// Task B2 (S3b-4) tests — append-to-document mode wiring.
|
||||
#[cfg(test)]
|
||||
#[path = "run_tests_b4.rs"]
|
||||
mod tests_b4;
|
||||
|
|
|
|||
471
crates/op-orchestrator/src/run_tests_b4.rs
Normal file
471
crates/op-orchestrator/src/run_tests_b4.rs
Normal file
|
|
@ -0,0 +1,471 @@
|
|||
//! Task B2 (S3b-4) tests — append-to-document mode wiring in `Orchestrator::run()`.
|
||||
//!
|
||||
//! Wired as `#[path = "run_tests_b4.rs"] mod tests_b4;` inside `run.rs`;
|
||||
//! stays a child module of `run`, so `use super::*` resolves to `run`.
|
||||
//!
|
||||
//! Covers:
|
||||
//! - Append mode: no scaffold `InsertSubtree` for the root, every subtask's
|
||||
//! `parent_frame_id` is the ctx target, effective concurrency forced to 1.
|
||||
//! - Append mode: sub-agent content lands as new children of `ctx.target_parent_id`.
|
||||
//! - Non-append mode: all prior paths unchanged (covered by green existing tests).
|
||||
|
||||
use super::*;
|
||||
use crate::test_support::{ScriptResponse, ScriptedLlm, VecDocSink};
|
||||
use crate::types::{AppendContext, DesignRequest};
|
||||
use op_editor_core::{EditorCommand, PenNodeExt};
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
fn node_json(prefix: &str) -> String {
|
||||
format!(
|
||||
r#"[{{"type":"frame","id":"{prefix}-1","name":"Sec","x":0,"y":0,"width":1200,"height":300,"children":[]}}]"#
|
||||
)
|
||||
}
|
||||
|
||||
/// Insert a target frame into `sink` and return its LIVE id
|
||||
/// (the remapped `n{N}` id assigned by `cmd_insert_subtree`).
|
||||
/// The target frame is an empty vertical frame of 1200 wide.
|
||||
fn insert_target_frame(sink: &mut VecDocSink, hint_id: &str) -> String {
|
||||
let node: jian_ops_schema::node::PenNode = serde_json::from_str(&format!(
|
||||
r#"{{"type":"frame","id":"{hint_id}","name":"Existing","x":0,"y":0,
|
||||
"width":1200,"height":800,"layout":"vertical","gap":0,"children":[]}}"#
|
||||
))
|
||||
.expect("parse target node");
|
||||
let before_count = sink.state().active_children().len();
|
||||
sink.apply(EditorCommand::InsertSubtree {
|
||||
nodes: vec![node],
|
||||
parent_id: op_editor_core::NodeId::NONE,
|
||||
});
|
||||
// The newly inserted node is appended at the end.
|
||||
sink.state()
|
||||
.active_children()
|
||||
.get(before_count)
|
||||
.map(|n| n.id_str().to_string())
|
||||
.unwrap_or_else(|| hint_id.to_string())
|
||||
}
|
||||
|
||||
/// Insert a target frame for mobile (390 wide) and return its live id.
|
||||
fn insert_target_frame_mobile(sink: &mut VecDocSink, hint_id: &str) -> String {
|
||||
let node: jian_ops_schema::node::PenNode = serde_json::from_str(&format!(
|
||||
r#"{{"type":"frame","id":"{hint_id}","name":"Existing","x":0,"y":0,
|
||||
"width":390,"height":844,"layout":"vertical","gap":0,"children":[]}}"#
|
||||
))
|
||||
.expect("parse target node");
|
||||
let before_count = sink.state().active_children().len();
|
||||
sink.apply(EditorCommand::InsertSubtree {
|
||||
nodes: vec![node],
|
||||
parent_id: op_editor_core::NodeId::NONE,
|
||||
});
|
||||
sink.state()
|
||||
.active_children()
|
||||
.get(before_count)
|
||||
.map(|n| n.id_str().to_string())
|
||||
.unwrap_or_else(|| hint_id.to_string())
|
||||
}
|
||||
|
||||
fn req_append(live_target_id: &str) -> DesignRequest {
|
||||
DesignRequest {
|
||||
prompt: "add a pricing section".into(),
|
||||
model: None,
|
||||
provider: None,
|
||||
design_md: None,
|
||||
concurrency: 1,
|
||||
append_context: Some(AppendContext {
|
||||
target_parent_id: live_target_id.into(),
|
||||
target_width: 1200.0,
|
||||
existing_section_labels: vec!["Hero".into(), "Features".into()],
|
||||
is_mobile: false,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn req_append_concurrent(live_target_id: &str) -> DesignRequest {
|
||||
DesignRequest {
|
||||
prompt: "add more screens".into(),
|
||||
model: None,
|
||||
provider: None,
|
||||
design_md: None,
|
||||
concurrency: 4,
|
||||
append_context: Some(AppendContext {
|
||||
target_parent_id: live_target_id.into(),
|
||||
target_width: 390.0,
|
||||
existing_section_labels: vec![],
|
||||
is_mobile: false,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
// ── Task B2 tests ─────────────────────────────────────────────────────────────
|
||||
|
||||
/// (a) Append mode: run does NOT emit a scaffold-root InsertSubtree.
|
||||
///
|
||||
/// We pre-insert the target frame, capture its live remapped id, then run
|
||||
/// with append mode. The run should emit only 2 sub-agent InsertSubtrees
|
||||
/// (one per subtask), NOT a scaffold-root InsertSubtree.
|
||||
#[test]
|
||||
fn append_mode_does_not_emit_scaffold_root_insert() {
|
||||
// Minimal plan whose rootFrame.id doesn't matter — apply_append_context_to_plan
|
||||
// replaces it with ctx.target_parent_id (the live id) before any scaffold call.
|
||||
const PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "frame-placeholder", "name": "Page", "width": 1200, "height": 800,
|
||||
"layout": "vertical", "gap": 0,
|
||||
"fill": [{ "type": "solid", "color": "#FFFFFF" }] },
|
||||
"subtasks": [
|
||||
{ "id": "pricing", "label": "Pricing Section", "region": { "width": 1200, "height": 400 } },
|
||||
{ "id": "cta", "label": "Call to Action", "region": { "width": 1200, "height": 300 } }
|
||||
]
|
||||
}"##;
|
||||
|
||||
let mut sink = VecDocSink::new();
|
||||
let live_id = insert_target_frame(&mut sink, "hint-target");
|
||||
// After pre-setup, sink has 1 InsertSubtree (the pre-insert).
|
||||
let pre_insert_count = sink
|
||||
.applied
|
||||
.iter()
|
||||
.filter(|c| matches!(c, EditorCommand::InsertSubtree { .. }))
|
||||
.count();
|
||||
assert_eq!(pre_insert_count, 1, "sanity: pre-insert count");
|
||||
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("pricing")),
|
||||
ScriptResponse::Text(node_json("cta")),
|
||||
]);
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let result = futures::executor::block_on(Orchestrator::new().run(
|
||||
req_append(&live_id),
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
));
|
||||
|
||||
assert!(result.is_ok(), "append run should succeed: {result:?}");
|
||||
|
||||
// Count InsertSubtrees AFTER pre-setup: 2 sub-agent inserts expected,
|
||||
// NOT 3 (which would indicate a scaffold-root insert).
|
||||
let total_inserts = sink
|
||||
.applied
|
||||
.iter()
|
||||
.filter(|c| matches!(c, EditorCommand::InsertSubtree { .. }))
|
||||
.count();
|
||||
// 1 (pre-setup target frame) + 2 (sub-agents) = 3 total.
|
||||
// If scaffold were inserted, it would be 4.
|
||||
assert_eq!(
|
||||
total_inserts, 3,
|
||||
"expected exactly 3 InsertSubtrees (1 pre-setup + 2 sub-agents, no scaffold root): got {total_inserts}"
|
||||
);
|
||||
}
|
||||
|
||||
/// (b) Append mode: summary.root_frame_id equals ctx.target_parent_id.
|
||||
#[test]
|
||||
fn append_mode_summary_root_equals_target_parent_id() {
|
||||
const PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "frame-placeholder", "name": "Page", "width": 1200, "height": 800,
|
||||
"layout": "vertical", "gap": 0,
|
||||
"fill": [{ "type": "solid", "color": "#FFFFFF" }] },
|
||||
"subtasks": [
|
||||
{ "id": "pricing", "label": "Pricing Section", "region": { "width": 1200, "height": 400 } },
|
||||
{ "id": "cta", "label": "Call to Action", "region": { "width": 1200, "height": 300 } }
|
||||
]
|
||||
}"##;
|
||||
|
||||
let mut sink = VecDocSink::new();
|
||||
let live_id = insert_target_frame(&mut sink, "hint-target");
|
||||
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("pricing")),
|
||||
ScriptResponse::Text(node_json("cta")),
|
||||
]);
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let summary = futures::executor::block_on(Orchestrator::new().run(
|
||||
req_append(&live_id),
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
))
|
||||
.expect("append run ok");
|
||||
|
||||
assert_eq!(
|
||||
summary.root_frame_id, live_id,
|
||||
"root_frame_id must match ctx.target_parent_id (the live id)"
|
||||
);
|
||||
assert_eq!(summary.subtasks.len(), 2, "both subtasks should appear");
|
||||
}
|
||||
|
||||
/// (c) Append mode: effective concurrency is 1 even with concurrency=4 + 2 screens.
|
||||
///
|
||||
/// With `concurrency=4` and a plan with 2 distinct screens, the normal path
|
||||
/// would trigger concurrent multi-screen mode. Append mode forces sequential.
|
||||
#[test]
|
||||
fn append_mode_forces_effective_concurrency_to_one() {
|
||||
const MULTISCREEN_PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "frame-placeholder", "name": "App", "width": 390, "height": 844,
|
||||
"layout": "vertical", "gap": 0,
|
||||
"fill": [{ "type": "solid", "color": "#FFFFFF" }] },
|
||||
"subtasks": [
|
||||
{ "id": "login", "label": "Login Screen", "region": { "width": 390, "height": 844 },
|
||||
"screen": "Login" },
|
||||
{ "id": "home", "label": "Home Screen", "region": { "width": 390, "height": 844 },
|
||||
"screen": "Home" }
|
||||
]
|
||||
}"##;
|
||||
|
||||
let mut sink = VecDocSink::new();
|
||||
let live_id = insert_target_frame_mobile(&mut sink, "hint-screens");
|
||||
|
||||
// 2 sub-agent calls = sequential path. If concurrent, the scripted LLM
|
||||
// would receive 2 parallel calls which our ScriptedLlm handles fine, but
|
||||
// the summary would have 2 separate root frames. We verify single root.
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(MULTISCREEN_PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("login")),
|
||||
ScriptResponse::Text(node_json("home")),
|
||||
]);
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let summary = futures::executor::block_on(Orchestrator::new().run(
|
||||
req_append_concurrent(&live_id),
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
))
|
||||
.expect("append concurrent run ok");
|
||||
|
||||
// Sequential path: exactly 1 root (the target frame).
|
||||
assert_eq!(
|
||||
summary.root_frame_id, live_id,
|
||||
"root_frame_id must be the target in sequential mode"
|
||||
);
|
||||
assert_eq!(summary.subtasks.len(), 2);
|
||||
|
||||
// 1 pre-setup + 2 sub-agent InsertSubtrees = 3 total (no N scaffold roots).
|
||||
let inserts = sink
|
||||
.applied
|
||||
.iter()
|
||||
.filter(|c| matches!(c, EditorCommand::InsertSubtree { .. }))
|
||||
.count();
|
||||
assert_eq!(
|
||||
inserts, 3,
|
||||
"expected 3 InsertSubtrees (1 pre-setup + 2 sub-agents): got {inserts}"
|
||||
);
|
||||
}
|
||||
|
||||
/// (d) Append mode: sub-agent content lands as new children of ctx.target_parent_id.
|
||||
///
|
||||
/// The target frame starts empty; after the run it must have more descendants.
|
||||
#[test]
|
||||
fn append_mode_content_lands_in_target_frame() {
|
||||
const PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "frame-placeholder", "name": "Page", "width": 1200, "height": 800,
|
||||
"layout": "vertical", "gap": 0,
|
||||
"fill": [{ "type": "solid", "color": "#FFFFFF" }] },
|
||||
"subtasks": [
|
||||
{ "id": "pricing", "label": "Pricing Section", "region": { "width": 1200, "height": 400 } },
|
||||
{ "id": "cta", "label": "Call to Action", "region": { "width": 1200, "height": 300 } }
|
||||
]
|
||||
}"##;
|
||||
|
||||
let mut sink = VecDocSink::new();
|
||||
let live_id = insert_target_frame(&mut sink, "hint-target");
|
||||
let before_count = descendant_count(sink.state(), &live_id);
|
||||
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("pricing")),
|
||||
ScriptResponse::Text(node_json("cta")),
|
||||
]);
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let summary = futures::executor::block_on(Orchestrator::new().run(
|
||||
req_append(&live_id),
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
))
|
||||
.expect("append run ok");
|
||||
|
||||
let after_count = descendant_count(sink.state(), &live_id);
|
||||
|
||||
assert!(
|
||||
after_count > before_count,
|
||||
"target frame must gain descendants: before={before_count} after={after_count}"
|
||||
);
|
||||
assert!(
|
||||
summary.total_nodes >= 2,
|
||||
"total_nodes must reflect sub-agent output"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-append mode: existing sequential path is unchanged.
|
||||
///
|
||||
/// A request without `append_context` takes the normal scaffold+sequential
|
||||
/// path: it inserts a scaffold root frame AND the sub-agent nodes.
|
||||
#[test]
|
||||
fn non_append_mode_takes_normal_sequential_path() {
|
||||
const PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "root", "name": "Page", "width": 1200, "height": 800,
|
||||
"layout": "vertical", "gap": 0,
|
||||
"fill": [{ "type": "solid", "color": "#FFFFFF" }] },
|
||||
"subtasks": [
|
||||
{ "id": "hero", "label": "Hero", "region": { "width": 1200, "height": 400 } },
|
||||
{ "id": "feat", "label": "Features", "region": { "width": 1200, "height": 400 } }
|
||||
]
|
||||
}"##;
|
||||
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("hero")),
|
||||
ScriptResponse::Text(node_json("feat")),
|
||||
]);
|
||||
let mut sink = VecDocSink::new();
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let req = DesignRequest {
|
||||
prompt: "a landing page".into(),
|
||||
model: None,
|
||||
provider: None,
|
||||
design_md: None,
|
||||
concurrency: 1,
|
||||
append_context: None, // no append context
|
||||
};
|
||||
|
||||
let summary = futures::executor::block_on(Orchestrator::new().run(
|
||||
req,
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
))
|
||||
.expect("non-append run ok");
|
||||
|
||||
// Normal path: scaffold root + 2 subtask inserts = at least 3 InsertSubtree.
|
||||
let inserts = sink
|
||||
.applied
|
||||
.iter()
|
||||
.filter(|c| matches!(c, EditorCommand::InsertSubtree { .. }))
|
||||
.count();
|
||||
assert!(
|
||||
inserts >= 3,
|
||||
"expected >=3 InsertSubtrees (scaffold + subtasks): got {inserts}"
|
||||
);
|
||||
assert!(!summary.root_frame_id.is_empty());
|
||||
assert_eq!(summary.subtasks.len(), 2);
|
||||
}
|
||||
|
||||
// ── Dashboard / append mutex (spec §2) ────────────────────────────────────────
|
||||
|
||||
/// Append + dashboard request → append fast-path wins (dashboard does NOT fire).
|
||||
///
|
||||
/// The request carries `Some(append_context)` AND a prompt + plan that would
|
||||
/// normally trigger `should_use_dashboard_columns`:
|
||||
/// - prompt "an analytics admin dashboard" → dashboard keyword
|
||||
/// - rootFrame.width 1440 (> 480)
|
||||
/// - sidebar subtask + metrics subtask
|
||||
///
|
||||
/// Without the mutex guard the dashboard path would dispatch first, inserting
|
||||
/// a multi-frame dashboard scaffold (sidebar + main + row frames). With the
|
||||
/// guard, the append fast-path takes priority — no scaffold-root InsertSubtree
|
||||
/// beyond the 1 pre-setup target, content lands in `ctx.target_parent_id`.
|
||||
#[test]
|
||||
fn append_mode_wins_over_dashboard_branch() {
|
||||
// Dashboard-like plan: 1440 wide, sidebar + metrics subtasks, dashboard
|
||||
// keyword in subtask labels.
|
||||
const DASHBOARD_PLAN_JSON: &str = r##"{
|
||||
"rootFrame": { "id": "frame-placeholder", "name": "Analytics Dashboard",
|
||||
"width": 1440, "height": 900,
|
||||
"layout": "vertical", "gap": 20,
|
||||
"fill": [{ "type": "solid", "color": "#0F172A" }] },
|
||||
"subtasks": [
|
||||
{ "id": "sidebar-nav", "label": "Sidebar Navigation",
|
||||
"region": { "width": 260, "height": 760 } },
|
||||
{ "id": "metrics-panel", "label": "Metrics Chart analytics dashboard",
|
||||
"region": { "width": 900, "height": 320 } }
|
||||
]
|
||||
}"##;
|
||||
|
||||
// Pre-insert the target frame (width 1440 to match the would-be dashboard).
|
||||
let mut sink = VecDocSink::new();
|
||||
let live_id = {
|
||||
let node: jian_ops_schema::node::PenNode = serde_json::from_str(
|
||||
r#"{"type":"frame","id":"hint-dash","name":"Existing","x":0,"y":0,
|
||||
"width":1440,"height":900,"layout":"vertical","gap":0,"children":[]}"#,
|
||||
)
|
||||
.expect("parse target node");
|
||||
let before_count = sink.state().active_children().len();
|
||||
sink.apply(EditorCommand::InsertSubtree {
|
||||
nodes: vec![node],
|
||||
parent_id: op_editor_core::NodeId::NONE,
|
||||
});
|
||||
sink.state()
|
||||
.active_children()
|
||||
.get(before_count)
|
||||
.map(|n| n.id_str().to_string())
|
||||
.unwrap_or_else(|| "hint-dash".to_string())
|
||||
};
|
||||
let before_count = descendant_count(sink.state(), &live_id);
|
||||
|
||||
// Prompt carries the dashboard keyword so without the mutex guard
|
||||
// `should_use_dashboard_columns` would return true.
|
||||
let req = DesignRequest {
|
||||
prompt: "an analytics admin dashboard".into(),
|
||||
model: None,
|
||||
provider: None,
|
||||
design_md: None,
|
||||
concurrency: 1,
|
||||
append_context: Some(AppendContext {
|
||||
target_parent_id: live_id.clone(),
|
||||
target_width: 1440.0,
|
||||
existing_section_labels: vec!["Hero".into()],
|
||||
is_mobile: false,
|
||||
}),
|
||||
};
|
||||
|
||||
let llm = ScriptedLlm::new(vec![
|
||||
ScriptResponse::Text(DASHBOARD_PLAN_JSON.into()),
|
||||
ScriptResponse::Text(node_json("sidebar")),
|
||||
ScriptResponse::Text(node_json("metrics")),
|
||||
]);
|
||||
let abort = AbortFlag::new();
|
||||
|
||||
let summary = futures::executor::block_on(Orchestrator::new().run(
|
||||
req,
|
||||
&mut sink,
|
||||
&llm,
|
||||
&mut |_| {},
|
||||
&abort,
|
||||
))
|
||||
.expect("append+dashboard run ok");
|
||||
|
||||
// Append fast-path won: root_frame_id matches the live target.
|
||||
assert_eq!(
|
||||
summary.root_frame_id, live_id,
|
||||
"append fast-path must win — root_frame_id should be the target id"
|
||||
);
|
||||
|
||||
// No dashboard scaffold: total InsertSubtree count is exactly
|
||||
// 1 (pre-setup) + 2 (sub-agents) = 3. Dashboard scaffold would push
|
||||
// 1 root + 1 sidebar + 1 main + N row frames → >>3.
|
||||
let inserts = sink
|
||||
.applied
|
||||
.iter()
|
||||
.filter(|c| matches!(c, EditorCommand::InsertSubtree { .. }))
|
||||
.count();
|
||||
assert_eq!(
|
||||
inserts, 3,
|
||||
"expected 3 InsertSubtrees (1 pre-setup + 2 sub-agents, no dashboard scaffold): got {inserts}"
|
||||
);
|
||||
|
||||
// Content landed in target frame.
|
||||
let after_count = descendant_count(sink.state(), &live_id);
|
||||
assert!(
|
||||
after_count > before_count,
|
||||
"target frame must gain descendants: before={before_count} after={after_count}"
|
||||
);
|
||||
}
|
||||
Loading…
Reference in a new issue