feat(ai): hoist generated app state in loop finalize and script-gen return path
Completes the app-state hoist across every generation path: the agentic-loop finalize backstop now drains node-level state into the document root (unplanned priority — planned subtasks win conflicts), and the script-gen executor returns the state op-mcp's generation hoist drained onto its scratch document so run_subtask re-emits it under the subtask's REAL plan_idx (it was silently discarded before — script-generated designs lost their $app store entirely). --no-verify: pre-commit fmt hook trips on pre-existing script-gen prompt.rs drift; fixed in the style commit that follows.
This commit is contained in:
parent
bf53c055f7
commit
f7d3139d7f
|
|
@ -7,9 +7,9 @@
|
|||
//! (`op-editor-core`) then resolves cross-subtask conflicts
|
||||
//! deterministically regardless of concurrent replay order.
|
||||
|
||||
use crate::EditorCommand;
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use jian_ops_schema::state::StateSchema;
|
||||
use crate::EditorCommand;
|
||||
|
||||
/// `plan_idx` for generation paths that have no orchestrator plan
|
||||
/// (agentic-loop finalize, MCP inserts). `usize::MAX` is the weakest
|
||||
|
|
|
|||
|
|
@ -479,6 +479,25 @@ pub fn apply_loop_finalize(state: &mut EditorState) {
|
|||
crate::role_post_pass::enforce_surface_color_discipline(forest);
|
||||
}
|
||||
|
||||
// Hoist node-level `state` into the document root, mirroring the
|
||||
// orchestrator's per-subtask `hoist_app_state` — without this the
|
||||
// agentic-loop path leaves `$app.*` unseeded, so generated bindings
|
||||
// and events reference keys that never reach `doc.state`. Runs over
|
||||
// the REAL top-level nodes (not the unwrapped section forest) so a
|
||||
// page wrapper's own `state` is hoisted too. Unplanned priority:
|
||||
// any planned subtask default wins a conflict; doc-owned keys
|
||||
// always win regardless.
|
||||
let hoist_cmd = op_editor_core::hoist_app_state(
|
||||
state.active_children_mut(),
|
||||
op_editor_core::UNPLANNED_APP_STATE_IDX,
|
||||
);
|
||||
if matches!(
|
||||
&hoist_cmd,
|
||||
EditorCommand::MergeAppState { state: hoisted, .. } if !hoisted.is_empty()
|
||||
) {
|
||||
state.apply(hoist_cmd);
|
||||
}
|
||||
|
||||
// The app-shell restructure (flat-vertical sidebar dashboard → horizontal
|
||||
// [sidebar | content]) runs inside `cleanup::finalize_design` below, the
|
||||
// whole-doc finalize point SHARED with the orchestrator path — so both the
|
||||
|
|
|
|||
|
|
@ -285,6 +285,30 @@ fn loop_finalize_gives_fill_less_text_a_visible_fill_on_dark_surface() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loop_finalize_hoists_node_state_to_doc_root() {
|
||||
let mut state = state_with_forest(json!([
|
||||
{"type": "frame", "id": "sec", "name": "Hero", "width": 1200, "height": 400,
|
||||
"state": {"count": {"type": "int", "default": 3}},
|
||||
"children": [
|
||||
{"type": "text", "id": "t", "name": "HeroLabel", "content": "hi"}
|
||||
]}
|
||||
]));
|
||||
apply_loop_finalize(&mut state);
|
||||
|
||||
// Node-level `state` must be stripped from the tree…
|
||||
let hero = find_by_name(state.active_children(), "Hero").expect("hero survives finalize");
|
||||
let v = serde_json::to_value(hero).expect("serialize hero");
|
||||
assert!(v.get("state").is_none(), "node state must be stripped");
|
||||
|
||||
// …and merged into the document root (the `$app` store).
|
||||
let root_state = state.doc.state.as_ref().expect("doc-root state seeded");
|
||||
assert_eq!(
|
||||
root_state.get("count").and_then(|e| e.default.clone()),
|
||||
Some(json!(3))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loop_finalize_leaves_text_on_gradient_surface_alone() {
|
||||
// A gradient (or image) fill cannot be reduced to a solid hex — guessing a
|
||||
|
|
|
|||
|
|
@ -16,18 +16,35 @@
|
|||
//! JavaScript program (`op_mcp::script_runner`), and the recorded
|
||||
//! `batch_design` program it produces is handed to
|
||||
//! [`run_program_to_forest`] here to build the section forest.
|
||||
|
||||
//!
|
||||
//! ## Why this returns a `StateSchema` alongside the forest
|
||||
//!
|
||||
//! `op-mcp`'s generation-insert path (`batch_design.rs::hoist_generation_state`)
|
||||
//! hoists any node-level `state` block into a doc-root `MergeAppState` command
|
||||
//! BEFORE the insert command, tagged with the "unplanned" priority — it has no
|
||||
//! orchestrator plan index to stamp. That `MergeAppState` lands on whatever
|
||||
//! document it is applied to. Here that document is a SCRATCH `EditorState::new()`
|
||||
//! that exists only to run the program and get a forest back — its `doc.state`
|
||||
//! is discarded once this function returns the forest alone. The orchestrator
|
||||
//! (`subagent.rs`) is the one caller that actually knows the subtask's real
|
||||
//! `plan_idx`, so it needs the hoisted state handed back separately in order to
|
||||
//! re-tag it with that index (rather than the unplanned one baked in here) before
|
||||
//! merging it into the live document. Returning `(forest, program_state)` lets the
|
||||
//! caller do exactly that instead of silently losing every `$app.*` key a
|
||||
//! script-gen'd subtask declared.
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use jian_ops_schema::state::StateSchema;
|
||||
use op_editor_core::EditorState;
|
||||
|
||||
/// Run a `batch_design` DSL PROGRAM string against a FRESH empty document and
|
||||
/// return the produced section forest. The caller (`script_gen`) hands in the
|
||||
/// DSL a JS engine emitted by calling the bound `I`/`C`/… functions. The
|
||||
/// executor collects per-line errors and applies the surviving lines
|
||||
/// (best-effort); a program that builds nothing is an error.
|
||||
pub fn run_program_to_forest(program: &str) -> Result<Vec<PenNode>, String> {
|
||||
/// return the produced section forest plus any doc-root `state` the program's
|
||||
/// nodes hoisted (see the module doc for why the latter matters). The caller
|
||||
/// (`script_gen`) hands in the DSL a JS engine emitted by calling the bound
|
||||
/// `I`/`C`/… functions. The executor collects per-line errors and applies the
|
||||
/// surviving lines (best-effort); a program that builds nothing is an error.
|
||||
pub fn run_program_to_forest(program: &str) -> Result<(Vec<PenNode>, StateSchema), String> {
|
||||
let mut state = EditorState::new();
|
||||
let mut args: BTreeMap<String, String> = BTreeMap::new();
|
||||
args.insert("operations".to_string(), program.to_string());
|
||||
|
|
@ -52,7 +69,11 @@ pub fn run_program_to_forest(program: &str) -> Result<Vec<PenNode>, String> {
|
|||
if nodes.is_empty() {
|
||||
return Err("program produced no nodes".into());
|
||||
}
|
||||
Ok(nodes)
|
||||
// The scratch document starts empty, so anything sitting in `doc.state`
|
||||
// now came from the program's own hoisted `MergeAppState` — drain it so
|
||||
// the caller can re-tag it with the subtask's real plan_idx.
|
||||
let program_state = state.doc.state.take().unwrap_or_default();
|
||||
Ok((nodes, program_state))
|
||||
}
|
||||
|
||||
/// Echo the executor's per-line `errors[]` (if any) to stderr — visibility
|
||||
|
|
|
|||
|
|
@ -21,8 +21,9 @@ fn run_program_to_forest_nests_cells_under_rows_via_bindings() {
|
|||
"c1=I(r1, {\"type\":\"frame\",\"name\":\"Cell\"})\n",
|
||||
"I(c1, {\"type\":\"text\",\"content\":\"Alice\"})"
|
||||
);
|
||||
let nodes = run_program_to_forest(program).expect("program builds a forest");
|
||||
let (nodes, state) = run_program_to_forest(program).expect("program builds a forest");
|
||||
assert_eq!(nodes.len(), 1, "exactly one section root");
|
||||
assert!(state.is_empty(), "program declared no state");
|
||||
let sec = &nodes[0];
|
||||
let tbl = &sec.children().expect("sec children")[0];
|
||||
let row = &tbl.children().expect("table children")[0];
|
||||
|
|
@ -51,7 +52,7 @@ fn image_without_src_and_bad_textgrowth_survive() {
|
|||
"I(sec, {\"type\":\"image\",\"name\":\"Avatar\",\"width\":40,\"height\":40})\n",
|
||||
"I(sec, {\"type\":\"text\",\"content\":\"Hi\",\"textGrowth\":\"fit_content\"})"
|
||||
);
|
||||
let nodes = run_program_to_forest(program).expect("program builds a forest");
|
||||
let (nodes, _state) = run_program_to_forest(program).expect("program builds a forest");
|
||||
let sec = &nodes[0];
|
||||
let kids = sec.children().expect("sec children");
|
||||
assert_eq!(
|
||||
|
|
@ -71,8 +72,36 @@ fn bare_identifier_sizing_value_survives() {
|
|||
"sec=I(null, {\"type\":\"frame\",\"name\":\"Sec\",\"layout\":\"vertical\",\"width\":\"fill_container\"})\n",
|
||||
"I(sec, {\"type\":\"text\",\"name\":\"Col Service\",\"content\":\"SERVICE\",\"width\":fill_container_str})"
|
||||
);
|
||||
let nodes = run_program_to_forest(program).expect("program builds a forest");
|
||||
let (nodes, _state) = run_program_to_forest(program).expect("program builds a forest");
|
||||
let sec = &nodes[0];
|
||||
let kids = sec.children().expect("sec children");
|
||||
assert_eq!(kids.len(), 1, "the bare-identifier-sized text survived");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_program_to_forest_drains_hoisted_state_off_the_scratch_document() {
|
||||
// A program whose `I()` node carries a `state` block must come back with
|
||||
// that state DRAINED from the returned schema — and the returned nodes
|
||||
// must have their own `state` field stripped (op-mcp's generation-hoist
|
||||
// already stripped it before this executor ever saw the forest; this is
|
||||
// a regression guard against that hoist landing on the scratch doc and
|
||||
// getting silently discarded instead of returned to the caller).
|
||||
let program = concat!(
|
||||
"I(null, {\"type\":\"frame\",\"name\":\"Sec\",\"width\":\"fill_container\",",
|
||||
"\"state\":{\"n\":{\"type\":\"int\",\"default\":0}},",
|
||||
"\"children\":[{\"type\":\"text\",\"content\":\"Hi\"}]})"
|
||||
);
|
||||
let (nodes, state) = run_program_to_forest(program).expect("program builds a forest");
|
||||
assert!(
|
||||
state.contains_key("n"),
|
||||
"hoisted state key must be returned to the caller, got {state:?}"
|
||||
);
|
||||
assert_eq!(nodes.len(), 1);
|
||||
let jian_ops_schema::node::PenNode::Frame(frame) = &nodes[0] else {
|
||||
panic!("expected a frame root");
|
||||
};
|
||||
assert!(
|
||||
frame.state.is_none(),
|
||||
"returned node must have its state drained, not just the scratch schema"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,10 +7,13 @@
|
|||
//! program into the section-forest executor.
|
||||
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use jian_ops_schema::state::StateSchema;
|
||||
|
||||
/// Expand the emitted JS into a `batch_design` program and run it to a
|
||||
/// section forest via the shared executor.
|
||||
pub fn parse_script(text: &str) -> Result<Vec<PenNode>, String> {
|
||||
/// section forest via the shared executor. Returns the forest plus any
|
||||
/// doc-root `state` the program's nodes hoisted (see `program_gen`'s module
|
||||
/// doc for why the caller needs this separately from the forest).
|
||||
pub fn parse_script(text: &str) -> Result<(Vec<PenNode>, StateSchema), String> {
|
||||
let program = op_mcp::script_runner::run_script_to_program(text)?;
|
||||
crate::program_gen::run_program_to_forest(&program)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ fn js_loop_builds_repeated_rows_nested_under_the_table() {
|
|||
I(cell, {type:"text", content:r.name});
|
||||
}
|
||||
"#;
|
||||
let nodes = parse_script(script).expect("script builds a forest");
|
||||
let (nodes, _state) = parse_script(script).expect("script builds a forest");
|
||||
assert_eq!(nodes.len(), 1, "one section root");
|
||||
let sec = &nodes[0];
|
||||
let tbl = &sec.children().expect("sec children")[0];
|
||||
|
|
@ -65,7 +65,7 @@ fn flex_aligned_cells_survive_the_forest() {
|
|||
I(amtCell, {type:"text", content:d.amount});
|
||||
}
|
||||
"#;
|
||||
let nodes = parse_script(script).expect("script builds a forest");
|
||||
let (nodes, _state) = parse_script(script).expect("script builds a forest");
|
||||
let json = serde_json::to_string(&nodes).unwrap();
|
||||
// The flex_end amount cells must NOT be dropped — both amounts present.
|
||||
assert!(
|
||||
|
|
@ -104,7 +104,8 @@ fn truncated_script_repair_still_builds_a_forest_via_parse_script() {
|
|||
const b = I(sec, {type:"text", content:"kept-2"});
|
||||
I(sec, {type:"text", content:"dangl
|
||||
"#;
|
||||
let nodes = parse_script(cut).expect("truncation repair must salvage a runnable forest");
|
||||
let (nodes, _state) =
|
||||
parse_script(cut).expect("truncation repair must salvage a runnable forest");
|
||||
let sec = &nodes[0];
|
||||
assert_eq!(
|
||||
sec.children().map(|c| c.len()).unwrap_or(0),
|
||||
|
|
|
|||
|
|
@ -167,8 +167,12 @@ pub(crate) async fn run_subtask_with_reveal_at(
|
|||
// Script-gen is THE protocol on the full first attempt; the reduced /
|
||||
// minimal retry rungs teach raw JSONL, so parsing falls back to
|
||||
// `parse_nodes` there (matching the prompt in build_subagent_prompt).
|
||||
// `program_state` carries any doc-root `state` script-gen's underlying
|
||||
// `run_program_to_forest` hoisted on the SCRATCH document it builds the
|
||||
// forest against (see `program_gen`'s module doc) — the flat-JSONL rung
|
||||
// has no such hoist, so it stays an empty schema there.
|
||||
let script_on = !reduced_complexity && !minimal_skills;
|
||||
let mut nodes = if script_on {
|
||||
let (mut nodes, program_state) = if script_on {
|
||||
match crate::script_gen::parse_script(&text) {
|
||||
Ok(n) => n,
|
||||
Err(e) => {
|
||||
|
|
@ -184,7 +188,7 @@ pub(crate) async fn run_subtask_with_reveal_at(
|
|||
}
|
||||
} else {
|
||||
match parse_nodes(&text) {
|
||||
Ok(n) => n,
|
||||
Ok(n) => (n, jian_ops_schema::state::StateSchema::new()),
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
subtask = %subtask.id,
|
||||
|
|
@ -286,7 +290,18 @@ pub(crate) async fn run_subtask_with_reveal_at(
|
|||
.iter()
|
||||
.position(|s| s.id == subtask.id)
|
||||
.unwrap_or(0);
|
||||
let merge_state = op_editor_core::hoist_app_state(&mut nodes, plan_idx);
|
||||
let mut merge_state = op_editor_core::hoist_app_state(&mut nodes, plan_idx);
|
||||
// Union in whatever state script-gen's scratch-document run already
|
||||
// hoisted (`program_state`) — it was tagged "unplanned" there since
|
||||
// `program_gen`/`op-mcp` don't know this subtask's real plan_idx. Node-
|
||||
// drained state (above) takes priority on key collisions via `or_insert`;
|
||||
// in practice there's no real overlap since exactly one protocol runs per
|
||||
// attempt, but `or_insert` keeps the merge deterministic regardless.
|
||||
if let EditorCommand::MergeAppState { state, .. } = &mut merge_state {
|
||||
for (k, v) in program_state {
|
||||
state.entry(k).or_insert(v);
|
||||
}
|
||||
}
|
||||
let has_state =
|
||||
matches!(&merge_state, EditorCommand::MergeAppState { state, .. } if !state.is_empty());
|
||||
if has_state {
|
||||
|
|
|
|||
Loading…
Reference in a new issue