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

feat(agent): judge a deck board by its geometry as well as the prompt

Browse source 
Deck centring fired only when the prompt said "deck", so the agentic
loop — which has no prompt to read — left every board top-stacked with a
blank lower half. The geometric half of the judgement is now unioned in
at the point of use, covering the paths that have no request to consult.

Centring is an intent-tier move, though: an asymmetric board can be
exactly what the author composed, and the explicit-`justifyContent`
guard does not catch an author who simply placed content and never set a
distribution. So the geometric half is gated on the roots being this
run's own output, which the fresh and append paths can prove and the
whole-document finalize cannot. The prompt half stays ungated — a user
who asked for a deck stated the intent themselves.

Claude-Session: https://claude.ai/code/session_01FqKQqNj8exYwopGDpYUU7x
This commit is contained in:
Fini 2026-08-09 19:57:11 +08:00
parent 4f124b0b1d
commit 53fc40f305
6 changed files with 349 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

106
crates/op-orchestrator/src/cleanup.rs
View file

@ -26,6 +26,8 @@ use op_editor_core::{
mod cleanup_desktop_dashboard;
#[path = "cleanup_mobile_chrome.rs"]
mod cleanup_mobile_chrome;
#[path = "cleanup_root_patches.rs"]
mod cleanup_root_patches;
pub(crate) use cleanup_mobile_chrome::{
anchor_bottom_nav_last_for_all_roots, repair_mobile_structural_chrome_for_all_roots,
};
@ -54,6 +56,7 @@ use cleanup_bottom_nav_repairs::*;
use cleanup_clip_row_stroke::*;
use cleanup_container_geometry::*;
use cleanup_root_and_nav::*;
use cleanup_root_patches::*;
use cleanup_root_transform::*;
use cleanup_section_sizing::*;
@ -257,10 +260,31 @@ fn find_root<'a>(state: &'a EditorState, root_id: &str) -> Option<&'a PenNode> {
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
pub(crate) struct CleanupPolicy {
pub(crate) preserve_requested_root_height: bool,
/// Deck boards are fixed 16:9 surfaces. Their content is centred rather
/// than left to stack from the top edge, which on a 1080-tall board reads
/// as a half-empty slide.
/// The REQUEST asked for a deck (prompt keywords, via
/// `plan_normalize::NormInfo`). Deck boards are fixed 16:9 surfaces, so
/// their content is centred rather than left to stack from the top edge,
/// which on a 1080-tall board reads as a half-empty slide.
///
/// This flag is only HALF the judgement — see [`root_is_deck_board`], the
/// geometric half, which is unioned in at the point of use and covers
/// every path that has no prompt to read.
pub(crate) is_deck: bool,
/// `root_ids` are roots THIS RUN produced, not pre-existing content the
/// user may have arranged by hand.
///
/// Gates the geometric half of the deck judgement only. Centring a board
/// is an intent-tier move: an asymmetric composition can be exactly what
/// the author wanted (a 70/30 board is good design, not a top-stacked
/// defect), and the "explicit `justifyContent`" guard does not catch an
/// author who simply placed their content and never set a distribution.
/// The prompt half is deliberately NOT gated on this — a user who typed
/// "PPT" stated the intent themselves.
///
/// Defaults to `false` so a caller that cannot prove provenance gets the
/// safe answer. The orchestrator's fresh and append paths both pass their
/// own inserted root ids and set it; `loop_finalize` passes every
/// top-level root in the document and cannot, so it leaves it false.
pub(crate) roots_are_run_output: bool,
}
/// 阶段 4 清理 pass —— 在全部 subtask 插入完成后运行。
@ -373,76 +397,6 @@ pub fn run_cleanup_passes_with_summary(
);
}
/// Centre a deck board's content on its fixed 16:9 surface.
///
/// A slide root is 1080 tall no matter how much content it holds, and its
/// sections hug their own height, so without this they stack from the top and
/// leave the lower half blank — measured on every generated deck.
///
/// Only `justifyContent` is written. Stretching the sections to fill instead
/// would distort whatever the model composed; centring changes where the block
/// sits, not what it is.
fn centre_deck_board_content(sink: &mut dyn DocSink, root_id: &str) {
let Some(root) = sink
.state()
.active_children()
.iter()
.find(|node| node.id_str() == root_id)
else {
return;
};
let value = serde_json::to_value(root).unwrap_or(serde_json::Value::Null);
// Respect an explicit distribution: a board that deliberately pushes
// content apart (space_between) or pins it low is a composition, not the
// default top-stack this repairs.
if value
.get("justifyContent")
.and_then(serde_json::Value::as_str)
.is_some_and(|mode| !mode.is_empty())
{
return;
}
let node_id = NodeId::new(root.id_str());
sink.apply(EditorCommand::PatchNodeData {
node_id,
patch_json: r#"{"justifyContent":"center"}"#.to_string(),
page_id: None,
});
}
/// Write the repaired root gap as a property patch.
///
/// Deliberately NOT an `apply_root_transform`: that rebuilds the subtree and
/// hands the root a fresh id, which is the right shape for passes that
/// restructure but wrong for setting one number — anything holding the root id
/// (the caller's `root_ids`, the loop's screen bookkeeping) would be left
/// pointing at a node that no longer exists.
fn patch_root_section_gap(sink: &mut dyn DocSink, root_id: &str) {
let Some(root) = sink
.state()
.active_children()
.iter()
.find(|node| node.id_str() == root_id)
else {
return;
};
let node_id = NodeId::new(root.id_str());
let Ok(mut value) = serde_json::to_value(root) else {
return;
};
if !crate::root_section_gap::fix_root_section_gap(&mut value) {
return;
}
let Some(gap) = value.get("gap") else {
return;
};
sink.apply(EditorCommand::PatchNodeData {
node_id,
patch_json: format!(r#"{{"gap":{gap}}}"#),
page_id: None,
});
}
/// Test-only alias so policy-dependent passes can be driven directly.
#[cfg(test)]
pub(crate) fn run_cleanup_passes_with_summary_and_policy_for_tests(
@ -560,7 +514,7 @@ fn run_cleanup_passes_with_summary_and_policy(
// reach it in the same state.
patch_root_section_gap(sink, &rid);
debug_probe_child_height(sink, &rid, "root_gap");
if policy.is_deck {
if policy.is_deck || (policy.roots_are_run_output && root_is_deck_board(sink, &rid)) {
centre_deck_board_content(sink, &rid);
}
// Text that resolves to ~1:1 against its own background is not
@ -825,3 +779,7 @@ mod tests_card_height_equalize;
#[cfg(test)]
#[path = "cleanup_desktop_dashboard_tests.rs"]
mod tests_desktop_dashboard;
#[cfg(test)]
#[path = "cleanup_deck_geometry_tests.rs"]
mod tests_deck_geometry;

191
crates/op-orchestrator/src/cleanup_deck_geometry_tests.rs Normal file
View file

@ -0,0 +1,191 @@
//! The geometric half of the deck judgement in cleanup (`root_is_deck_board`)
//! and the provenance gate in front of it.
//!
//! `CleanupPolicy::is_deck` comes from prompt keywords, so it is false on every
//! path that has no prompt — a board generated without anyone typing "PPT" got
//! none of the board treatment. The artboard itself can supply that verdict.
//!
//! But centring is intent-tier: an asymmetric board can be the composition the
//! author wanted, and "no explicit `justifyContent`" does not distinguish an
//! author who just placed their content from a model that top-stacked it. So
//! the geometric verdict only applies to roots the run itself produced
//! (`roots_are_run_output`), which the orchestrator's fresh and append paths
//! can prove and the whole-document loop finalize cannot.
use super::*;
use crate::test_support::VecDocSink;
use op_editor_core::{EditorCommand, NodeId};
/// The policy shape of a fresh/append orchestrator run: these roots are this
/// run's own output, so their geometry may drive intent-tier decisions.
fn run_output_policy() -> CleanupPolicy {
CleanupPolicy {
roots_are_run_output: true,
..Default::default()
}
}
/// A top-stacked board with no authored distribution — the shape
/// `centre_deck_board_content` exists to repair.
fn board(width: f64, height: f64) -> jian_ops_schema::node::PenNode {
serde_json::from_value(serde_json::json!({
"type": "frame",
"id": "board",
"name": "Cover",
"width": width,
"height": height,
"layout": "vertical",
"children": [
{"type": "frame", "id": "s1", "name": "Title", "width": "fill_container", "height": "fit_content"},
{"type": "frame", "id": "s2", "name": "Body", "width": "fill_container", "height": "fit_content"}
]
}))
.expect("fixture must deserialize as PenNode")
}
fn plan_for(root_id: &str, width: f64, height: f64) -> crate::plan::OrchestratorPlan {
crate::plan::OrchestratorPlan {
root_frame: crate::plan::RootFrameSpec {
id: root_id.to_string(),
name: "Root".into(),
width,
height,
layout: Some("vertical".into()),
gap: None,
padding: None,
fill: None,
},
subtasks: Vec::new(),
style_guide_name: None,
}
}
fn run_with(width: f64, height: f64, policy: CleanupPolicy) -> serde_json::Value {
let mut sink = VecDocSink::new();
sink.state.apply(EditorCommand::InsertSubtree {
nodes: vec![board(width, height)],
parent_id: NodeId::NONE,
page_id: None,
});
let root_id = sink.state.active_children()[0].id_str().to_string();
let mut summary = crate::repair_summary::RepairSummary::default();
crate::cleanup::run_cleanup_passes_with_summary_and_policy_for_tests(
&mut sink,
&plan_for(&root_id, width, height),
&[&root_id],
&mut summary,
policy,
);
serde_json::to_value(&sink.state.active_children()[0]).expect("serialize")
}
#[test]
fn a_board_this_run_produced_is_centred_even_when_no_prompt_said_deck() {
// The hole the geometric half closes: a user who asks for a 1920x1080
// 主视觉 without ever typing PPT still drew a board, and `is_deck` (prompt
// keywords) is false for them.
let root = run_with(1920.0, 1080.0, run_output_policy());
assert_eq!(
root["justifyContent"].as_str(),
Some("center"),
"the artboard alone must be enough on this run's own root: {root}"
);
}
#[test]
fn a_board_this_run_did_not_produce_is_left_alone() {
// The intent guard. A hand-arranged board reaching a whole-document
// finalize (the loop path, which passes every top-level root because it
// cannot tell which are its own) must not be re-composed: an asymmetric
// board can be exactly what its author wanted, and an author who simply
// placed their content never set a `justifyContent` for the other guard
// to notice.
let root = run_with(1920.0, 1080.0, CleanupPolicy::default());
assert_eq!(
root.get("justifyContent")
.and_then(serde_json::Value::as_str),
None,
"pre-existing content must keep its authored composition: {root}"
);
}
#[test]
fn the_prompt_half_is_not_gated_on_provenance() {
// Saying "PPT" IS the statement of intent, so the prompt half applies to
// whatever the run was pointed at — unchanged by the provenance gate.
let root = run_with(
1920.0,
1080.0,
CleanupPolicy {
is_deck: true,
..Default::default()
},
);
assert_eq!(root["justifyContent"].as_str(), Some("center"));
}
#[test]
fn a_page_is_not_centred_by_the_geometric_half() {
// The union must be strictly additive. A 1200-wide scrolling page is not
// a board at any height, and centring one would push its hero away from
// the top of the viewport.
let root = run_with(1200.0, 2400.0, run_output_policy());
assert_eq!(
root.get("justifyContent")
.and_then(serde_json::Value::as_str),
None,
"a page must be untouched by the deck branch: {root}"
);
}
#[test]
fn a_deck_wide_long_page_is_not_a_board() {
// Same aspect gate the budget override now uses: deck WIDTH without deck
// SHAPE is a long page. Reading it as a board would centre 2000px of
// content in a 2000px frame — visible, and wrong.
let root = run_with(1920.0, 2000.0, run_output_policy());
assert_eq!(
root.get("justifyContent")
.and_then(serde_json::Value::as_str),
None,
"1920x2000 is a long page, not a projector board: {root}"
);
}
#[test]
fn an_authored_distribution_still_wins_over_the_geometric_half() {
// The existing "explicit distribution is a composition" guard is what
// keeps this union off a board somebody arranged on purpose. It must hold
// when the deck verdict comes from geometry, not just from the prompt.
let mut sink = VecDocSink::new();
let tree: jian_ops_schema::node::PenNode = serde_json::from_value(serde_json::json!({
"type": "frame",
"id": "board",
"name": "Cover",
"width": 1920,
"height": 1080,
"layout": "vertical",
"justifyContent": "space_between",
"children": [
{"type": "frame", "id": "s1", "name": "Title", "width": "fill_container", "height": "fit_content"},
{"type": "frame", "id": "s2", "name": "Meta", "width": "fill_container", "height": "fit_content"}
]
}))
.expect("fixture must deserialize as PenNode");
sink.state.apply(EditorCommand::InsertSubtree {
nodes: vec![tree],
parent_id: NodeId::NONE,
page_id: None,
});
let root_id = sink.state.active_children()[0].id_str().to_string();
let mut summary = crate::repair_summary::RepairSummary::default();
crate::cleanup::run_cleanup_passes_with_summary_and_policy_for_tests(
&mut sink,
&plan_for(&root_id, 1920.0, 1080.0),
&[&root_id],
&mut summary,
run_output_policy(),
);
let root = serde_json::to_value(&sink.state.active_children()[0]).expect("serialize");
assert_eq!(root["justifyContent"].as_str(), Some("space_between"));
}

9
crates/op-orchestrator/src/cleanup_root_height_tests.rs
View file

@ -494,6 +494,9 @@ fn cleanup_policy_preserves_only_requested_fixed_root_height() {
CleanupPolicy {
is_deck: false,
preserve_requested_root_height: true,
// These fixtures stand in for an orchestrator run, whose
// root_ids are its own freshly inserted roots.
roots_are_run_output: true,
},
);
let preserved_height = preserved_sink
@ -601,6 +604,9 @@ fn a_deck_board_centres_its_content() {
CleanupPolicy {
is_deck: true,
preserve_requested_root_height: true,
// These fixtures stand in for an orchestrator run, whose
// root_ids are its own freshly inserted roots.
roots_are_run_output: true,
},
);
@ -653,6 +659,9 @@ fn a_deck_board_with_an_explicit_distribution_is_left_alone() {
CleanupPolicy {
is_deck: true,
preserve_requested_root_height: true,
// These fixtures stand in for an orchestrator run, whose
// root_ids are its own freshly inserted roots.
roots_are_run_output: true,
},
);

98
crates/op-orchestrator/src/cleanup_root_patches.rs Normal file
View file

@ -0,0 +1,98 @@
//! Root-level property patches the cleanup driver applies in place.
//!
//! Carved off `cleanup.rs` (spine + siblings, 800-line cap) as pure code
//! motion. What holds these three together is the SHAPE of the edit, not the
//! defect they fix: each reads one root, decides one property, and writes it
//! with a `PatchNodeData` — deliberately NOT an `apply_root_transform`, which
//! rebuilds the subtree and hands the root a fresh id. Anything holding the
//! root id (the caller's `root_ids`, the loop's screen bookkeeping) would be
//! left pointing at a node that no longer exists, so a pass that only sets a
//! number or a keyword must not use one.
use op_editor_core::{EditorCommand, NodeId, PenNodeExt};
use crate::types::DocSink;
/// The geometric half of the deck judgement, per root.
///
/// [`crate::cleanup::CleanupPolicy::is_deck`] answers "the REQUEST asked for a
/// deck", which is prompt keywords and therefore blind three ways: the agentic
/// loop has no prompt at all (its policy is `Default`, so `is_deck` was
/// permanently false), a scene template that reaches cleanup carries no
/// request, and a user who says "1920×1080 主视觉" without ever typing PPT
/// still drew a board. Unioned in at the point of use rather than baked into
/// the policy so the answer is taken PER ROOT — one document can hold a board
/// and a page, and a single run-wide flag cannot describe that.
pub(super) fn root_is_deck_board(sink: &dyn DocSink, root_id: &str) -> bool {
crate::geometry_validation::root_design_form(sink.state(), root_id).is_deck_board()
}
/// Centre a deck board's content on its fixed 16:9 surface.
///
/// A slide root is 1080 tall no matter how much content it holds, and its
/// sections hug their own height, so without this they stack from the top and
/// leave the lower half blank — measured on every generated deck.
///
/// Only `justifyContent` is written. Stretching the sections to fill instead
/// would distort whatever the model composed; centring changes where the block
/// sits, not what it is.
pub(super) fn centre_deck_board_content(sink: &mut dyn DocSink, root_id: &str) {
let Some(root) = sink
.state()
.active_children()
.iter()
.find(|node| node.id_str() == root_id)
else {
return;
};
let value = serde_json::to_value(root).unwrap_or(serde_json::Value::Null);
// Respect an explicit distribution: a board that deliberately pushes
// content apart (space_between) or pins it low is a composition, not the
// default top-stack this repairs.
if value
.get("justifyContent")
.and_then(serde_json::Value::as_str)
.is_some_and(|mode| !mode.is_empty())
{
return;
}
let node_id = NodeId::new(root.id_str());
sink.apply(EditorCommand::PatchNodeData {
node_id,
patch_json: r#"{"justifyContent":"center"}"#.to_string(),
page_id: None,
});
}
/// Write the repaired root gap as a property patch.
///
/// Deliberately NOT an `apply_root_transform`: that rebuilds the subtree and
/// hands the root a fresh id, which is the right shape for passes that
/// restructure but wrong for setting one number — anything holding the root id
/// (the caller's `root_ids`, the loop's screen bookkeeping) would be left
/// pointing at a node that no longer exists.
pub(super) fn patch_root_section_gap(sink: &mut dyn DocSink, root_id: &str) {
let Some(root) = sink
.state()
.active_children()
.iter()
.find(|node| node.id_str() == root_id)
else {
return;
};
let node_id = NodeId::new(root.id_str());
let Ok(mut value) = serde_json::to_value(root) else {
return;
};
if !crate::root_section_gap::fix_root_section_gap(&mut value) {
return;
}
let Some(gap) = value.get("gap") else {
return;
};
sink.apply(EditorCommand::PatchNodeData {
node_id,
patch_json: format!(r#"{{"gap":{gap}}}"#),
page_id: None,
});
}

17
crates/op-orchestrator/src/repair_scope.rs
View file

@ -47,7 +47,22 @@ pub(crate) fn finalize_appended_design(
target_roots: &[&str],
summary: &mut RepairSummary,
) {
crate::cleanup::finalize_design_with_summary(sink, plan, inserted_roots, summary);
// `inserted_roots` are this run's own (the doc comment above says so, and
// the empty-slice case is already a no-op), so the intent-tier passes may
// key off their geometry. `target_roots` — the user's existing frame —
// never reaches this driver: `contract_sweep` below runs a named
// contract-tier pair over it instead, which is what keeps an appended-into
// board from being re-composed.
crate::cleanup::finalize_design_with_summary_and_policy(
sink,
plan,
inserted_roots,
summary,
crate::cleanup::CleanupPolicy {
roots_are_run_output: true,
..Default::default()
},
);
let swept = contract_sweep(sink, target_roots, summary);
if swept {
summary.note(scope_note(inserted_roots.len(), target_roots.len()));

3
crates/op-orchestrator/src/run_orchestrator.rs
View file

@ -637,6 +637,9 @@ impl Orchestrator {
CleanupPolicy {
preserve_requested_root_height: norm.preserve_requested_root_height,
is_deck: norm.is_deck,
// `root_ids` here are the screen-group roots this run just
// inserted — every node under them is this run's own.
roots_are_run_output: true,
},
);
}