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

Merge branch 'v0.8.0-new' of github.com:ZSeven-W/openpencil into v0.8.0-new

Browse source
This commit is contained in:
Fini 2026-05-24 13:03:26 +08:00
parent 83067b42d9
commit 90e8b7ff0a
11 changed files with 1030 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -2627,6 +2627,7 @@ dependencies = [
"op-acp",
"op-ai",
"op-ai-skills",
"op-design-lint",
"op-editor-core",
"op-editor-ui",
"op-figma",

2
crates/op-design-lint/src/lib.rs
View file

@ -10,6 +10,7 @@ pub mod fixes;
pub mod issue;
pub mod node_mut;
pub mod node_util;
pub mod plan;
pub use color::{color_contrast, parse_hex_color, relative_luminance, Rgb};
pub use detectors::{
@ -21,3 +22,4 @@ pub use detectors::{
};
pub use fixes::{apply_fixes, detect_and_fix};
pub use issue::{FixProperty, FixReport, Issue, IssueCategory, IssueSeverity};
pub use plan::{detect_and_plan, PlannedAction, PlannedFix};

529
crates/op-design-lint/src/plan.rs Normal file
View file

@ -0,0 +1,529 @@
//! Editor-agnostic `PlannedFix` API — a plan for what `apply_fixes` would do,
//! expressed as a data type rather than a direct mutation.
//!
//! `detect_and_plan` runs the same filter logic that `apply_fixes` uses
//! (skip `Info`-severity issues, skip the no-op `Fill` property, skip
//! `Remove` on `status-bar`-role nodes), then emits a `PlannedFix` per
//! issue that would have been applied. The final document produced by
//! applying every `PlannedFix` is byte-identical to the final document
//! produced by `detect_and_fix` — this invariant is verified by the
//! equivalence tests in this module.
//!
//! Host adapters (e.g. `op-host-desktop::pre_validator`) translate each
//! `PlannedFix` to one or more `EditorCommand`s and apply them via
//! `DocSink::apply`. This design keeps `op-design-lint` editor-agnostic
//! and isolates the `FixProperty → EditorCommand` translation table to a
//! single host module.
use crate::issue::{FixProperty, IssueCategory, IssueSeverity};
use crate::node_util;
use jian_ops_schema::PenDocument;
/// An editor-agnostic description of a fix that `detect_and_fix` would
/// have applied to the document. The `PlannedAction` captures the full
/// mutation in a form that a host adapter can translate to `EditorCommand`s.
///
/// Structural equivalence invariant: applying all `PlannedFix`es from
/// `detect_and_plan(doc)` using the mutation primitives in `crate::node_mut`
/// produces a document structurally identical to calling `detect_and_fix(doc)`.
#[derive(Debug, Clone, PartialEq)]
pub struct PlannedFix {
/// The node the fix targets.
pub node_id: String,
/// Which detector category produced this fix — used for per-category
/// counts in `PreValidationResult`.
pub category: IssueCategory,
/// The concrete mutation to apply.
pub action: PlannedAction,
}
/// The concrete mutation encoded by a `PlannedFix`.
///
/// Variants cover every mutation path in `fixes::apply_fixes` today:
///
/// | `FixProperty` | `PlannedAction` variant | Notes |
/// |-----------------|--------------------------------|-------------------------------|
/// | `Remove` | `Remove` | detach node from parent |
/// | `Height` | `SetHeightFitContent` | only `"fit_content"` is valid |
/// | `Rotation` | `SetRotation(f64)` | degrees |
/// | `CornerRadius` | `SetCornerRadius(f32)` | uniform radius |
/// | `FontSize` | `SetFontSize(f32)` | doc-px |
/// | `Effects` | `ClearEffects` | set effects field to None |
/// | `Padding` | `SetPadding(Value)` | uniform or [L,T,R,B] array |
/// | `Stroke` | `SetStroke(Value)` | full PenStroke JSON object |
/// | `Fill` | *(filtered out — no-op)* | `apply_fixes` returns `false` |
#[derive(Debug, Clone, PartialEq)]
pub enum PlannedAction {
/// Remove the node from its parent (mirrors `fixes::apply_remove`).
Remove,
/// Set the node's height sizing to `fit_content` (text-explicit-height fix).
SetHeightFitContent,
/// Set the node's rotation to the given degrees.
SetRotation(f64),
/// Set the node's corner-radius to a uniform value.
SetCornerRadius(f32),
/// Set the node's font size.
SetFontSize(f32),
/// Clear the node's effects list (set to `None`).
ClearEffects,
/// Set the node's padding. The `Value` is either a single number
/// (uniform) or a 4-element array `[L, T, R, B]`.
SetPadding(serde_json::Value),
/// Set the node's stroke to the given `PenStroke`-compatible JSON
/// object (e.g. `{"thickness":1,"fill":[{"type":"solid","color":"#E2E8F0"}]}`).
SetStroke(serde_json::Value),
}
/// Detect issues over the active page and return a plan of fixes —
/// the set that `apply_fixes` would have ACTUALLY applied (Info-severity
/// issues and `Fill` no-ops are excluded from the plan, matching the
/// filter in `apply_fixes`).
///
/// The returned list is in the same order as `apply_fixes` would have
/// processed the issues: one `PlannedFix` per issue that passes the
/// filter AND whose suggested value is usable (e.g. `Height` only when
/// `suggested_value == "fit_content"`).
///
/// This function reads the doc but does NOT mutate it; it is safe to
/// call on the same doc before handing it to the adapter.
pub fn detect_and_plan(doc: &PenDocument) -> Vec<PlannedFix> {
// Locate the active-page root. Mirror the logic in `fixes::detect_and_fix`.
let root = match active_page_root(doc) {
Some(r) => r,
None => return Vec::new(),
};
let issues = crate::detectors::detect_all(root, doc);
let mut plan = Vec::new();
for issue in &issues {
// Mirror `apply_fixes`: skip Info-severity.
if issue.severity == IssueSeverity::Info {
continue;
}
if issue.property == FixProperty::Remove {
// Mirror `apply_remove`: skip status-bar-role nodes.
// Walk the page roots read-only to check the role before deciding.
let is_status_bar = active_page_nodes(doc)
.iter()
.find_map(|root_node| find_node(root_node, &issue.node_id))
.map(|n| node_util::role(n) == Some("status-bar"))
.unwrap_or(false);
if is_status_bar {
continue;
}
plan.push(PlannedFix {
node_id: issue.node_id.clone(),
category: issue.category,
action: PlannedAction::Remove,
});
continue;
}
// Translate `(property, suggested_value)` to a `PlannedAction`,
// mirroring the `set_property` dispatch in `fixes.rs`. Skip combinations
// that `set_property` returns `false` for (Fill, non-fit_content Height, etc.).
let action = match issue.property {
FixProperty::Height => {
if issue.suggested_value.as_str() == Some("fit_content") {
PlannedAction::SetHeightFitContent
} else {
continue; // set_property returns false; skip
}
}
FixProperty::Rotation => {
let v = match issue.suggested_value.as_f64() {
Some(v) => v,
None => continue,
};
PlannedAction::SetRotation(v)
}
FixProperty::CornerRadius => {
let v = match issue.suggested_value.as_f64() {
Some(v) => v as f32,
None => continue,
};
PlannedAction::SetCornerRadius(v)
}
FixProperty::FontSize => {
let v = match issue.suggested_value.as_f64() {
Some(v) => v as f32,
None => continue,
};
PlannedAction::SetFontSize(v)
}
FixProperty::Effects => PlannedAction::ClearEffects,
FixProperty::Padding => PlannedAction::SetPadding(issue.suggested_value.clone()),
FixProperty::Stroke => PlannedAction::SetStroke(issue.suggested_value.clone()),
// Fill is a documented no-op in apply_fixes — skip.
FixProperty::Fill => continue,
// Remove is handled above; this arm is unreachable.
FixProperty::Remove => continue,
};
plan.push(PlannedFix {
node_id: issue.node_id.clone(),
category: issue.category,
action,
});
}
plan
}
// ── Private helpers (mirror fixes.rs helpers) ────────────────────────────────
fn active_page_nodes(doc: &PenDocument) -> &[jian_ops_schema::node::PenNode] {
match doc.pages.as_ref().and_then(|pages| pages.first()) {
Some(page) => &page.children,
None => &doc.children,
}
}
fn active_page_root(doc: &PenDocument) -> Option<&jian_ops_schema::node::PenNode> {
active_page_nodes(doc).first()
}
fn find_node<'a>(
root: &'a jian_ops_schema::node::PenNode,
node_id: &str,
) -> Option<&'a jian_ops_schema::node::PenNode> {
if node_util::node_id(root) == node_id {
return Some(root);
}
node_util::children(root)
.iter()
.find_map(|child| find_node(child, node_id))
}
// ── Equivalence tests ────────────────────────────────────────────────────────
#[cfg(test)]
mod tests {
use super::*;
use crate::fixes::detect_and_fix;
use crate::node_mut;
use crate::node_util;
use jian_ops_schema::node::PenNode;
use jian_ops_schema::PenDocument;
use serde_json::Value;
fn doc(value: serde_json::Value) -> PenDocument {
serde_json::from_value(value).expect("fixture must deserialize as PenDocument")
}
/// Apply a single `PlannedAction` to the target node in the document —
/// mirrors what `apply_fixes` does via `set_property` / `apply_remove`,
/// reusing the same `node_mut` helpers so the equivalence test is
/// meaningful (not just checking the same code path twice).
fn apply_planned_fix_to_doc(doc: &mut PenDocument, fix: &PlannedFix) -> bool {
if fix.action == PlannedAction::Remove {
// Mirror `apply_remove` — detach from whichever root contains it.
for root in active_page_roots_mut(doc) {
if node_mut::detach_child(root, &fix.node_id) {
return true;
}
}
return false;
}
// Find the node mutably and apply the action.
for root in active_page_roots_mut(doc) {
if let Some(node) = find_node_mut(root, &fix.node_id) {
return apply_action_to_node(node, &fix.action);
}
}
false
}
fn apply_action_to_node(node: &mut PenNode, action: &PlannedAction) -> bool {
match action {
PlannedAction::SetHeightFitContent => node_mut::set_text_height_fit_content(node),
PlannedAction::SetRotation(v) => node_mut::set_rotation(node, *v),
PlannedAction::SetCornerRadius(v) => node_mut::set_corner_radius(node, *v),
PlannedAction::SetFontSize(v) => node_mut::set_font_size(node, *v),
PlannedAction::ClearEffects => node_mut::clear_effects(node),
PlannedAction::SetPadding(v) => apply_padding(node, v),
PlannedAction::SetStroke(v) => node_mut::set_stroke_from_json(node, v),
PlannedAction::Remove => unreachable!("Remove handled in apply_planned_fix_to_doc"),
}
}
fn apply_padding(node: &mut PenNode, suggested: &Value) -> bool {
if let Some(n) = suggested.as_f64() {
return node_mut::set_padding_uniform(node, n as f32);
}
if let Some(arr) = suggested.as_array() {
if arr.len() == 4 {
let v: Vec<f32> = arr
.iter()
.filter_map(|x| x.as_f64().map(|f| f as f32))
.collect();
if v.len() == 4 {
return node_mut::set_padding_ltrb(node, [v[0], v[1], v[2], v[3]]);
}
}
}
false
}
fn active_page_roots_mut(doc: &mut PenDocument) -> &mut [jian_ops_schema::node::PenNode] {
let has_page = doc
.pages
.as_ref()
.map(|pages| !pages.is_empty())
.unwrap_or(false);
if has_page {
doc.pages.as_mut().unwrap()[0].children.as_mut_slice()
} else {
doc.children.as_mut_slice()
}
}
fn find_node_mut<'a>(root: &'a mut PenNode, node_id: &str) -> Option<&'a mut PenNode> {
if node_util::node_id(root) == node_id {
return Some(root);
}
match root {
PenNode::Frame(n) => n.children.as_mut(),
PenNode::Group(n) => n.children.as_mut(),
PenNode::Rectangle(n) => n.children.as_mut(),
_ => None,
}?
.iter_mut()
.find_map(|child| find_node_mut(child, node_id))
}
/// Load the `invisible-container` fixture and assert that `detect_and_plan`
/// + apply-each-fix produces a document identical to `detect_and_fix`.
///
/// This fixture exercises `FixProperty::Stroke` (the `Warning`-severity
/// `light-on-light` node) and the `Info`-severity skip (the `dark-on-dark`
/// node must NOT be in the plan).
#[test]
fn equivalence_invisible_container() {
let raw = include_str!("../tests/fixtures/docs/invisible-container.json");
let doc_a: PenDocument = serde_json::from_str(raw).expect("parse");
let doc_b: PenDocument = serde_json::from_str(raw).expect("parse");
// Clone A: detect_and_fix (the reference path).
let mut clone_a = doc_a;
let report = detect_and_fix(&mut clone_a);
// Clone B: detect_and_plan + apply each fix via node_mut primitives.
let mut clone_b = doc_b;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(
report.total as usize, applied,
"fix counts must match: detect_and_fix={}, planned={}",
report.total, applied
);
assert_eq!(plan.len(), applied, "all planned fixes must apply");
assert_eq!(
clone_a, clone_b,
"detect_and_fix and detect_and_plan+apply must produce identical documents"
);
// Verify the Info-severity dark-on-dark node is NOT in the plan.
let dark_in_plan = plan.iter().any(|f| f.node_id == "dark-on-dark");
assert!(
!dark_in_plan,
"Info-severity nodes must not appear in the plan"
);
}
/// Load the `invisible-container-with-var` fixture (doc declares
/// `color-border` variable) and assert equivalence — exercises the
/// `$color-border` design-token reference path.
///
/// Regression guard: caught by stop-time review — `LintPreValidator`
/// previously dropped `$color-border` refs while reporting success
/// because `cmd_set_node_stroke_hex` strict-parsed the hex. The
/// op-design-lint side (this test) ensures `detect_and_plan + apply`
/// produces the same `$color-border`-stamped doc as `detect_and_fix`;
/// the host parity test confirms the same through `EditorCommand`.
#[test]
fn equivalence_invisible_container_with_var() {
let raw = include_str!("../tests/fixtures/docs/invisible-container-with-var.json");
let doc_a: PenDocument = serde_json::from_str(raw).expect("parse");
let doc_b: PenDocument = serde_json::from_str(raw).expect("parse");
// Clone A: detect_and_fix (the reference path).
let mut clone_a = doc_a;
let report = detect_and_fix(&mut clone_a);
// Clone B: detect_and_plan + apply each fix via node_mut primitives.
let mut clone_b = doc_b;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(report.total as usize, applied);
assert_eq!(plan.len(), applied);
assert_eq!(
clone_a, clone_b,
"var-ref stroke must round-trip identically"
);
// Verify the plan carries the $color-border ref (not a resolved hex).
let stroke_plan = plan
.iter()
.find(|f| f.node_id == "light-on-light")
.expect("light-on-light should be in the plan");
let value = match &stroke_plan.action {
PlannedAction::SetStroke(v) => v,
other => panic!("expected SetStroke, got {other:?}"),
};
let color = value
.get("fill")
.and_then(|f| f.as_array())
.and_then(|arr| arr.first())
.and_then(|entry| entry.get("color"))
.and_then(|c| c.as_str())
.expect("color field");
assert_eq!(
color, "$color-border",
"plan must preserve design-token ref, not resolve to hex"
);
}
/// Load the `text-explicit-height` fixture and assert equivalence.
///
/// This fixture exercises `FixProperty::Height` with `"fit_content"`.
#[test]
fn equivalence_text_explicit_height() {
let raw = include_str!("../tests/fixtures/docs/text-explicit-height.json");
let doc_a: PenDocument = serde_json::from_str(raw).expect("parse");
let doc_b: PenDocument = serde_json::from_str(raw).expect("parse");
let mut clone_a = doc_a;
let report = detect_and_fix(&mut clone_a);
let mut clone_b = doc_b;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(report.total as usize, applied);
assert_eq!(plan.len(), applied);
assert_eq!(clone_a, clone_b);
// The fit-text node with explicit `height: "fit_content"` must NOT appear
// (it already has the keyword, no issue is raised for it).
let fit_in_plan = plan.iter().any(|f| f.node_id == "fit-text");
assert!(
!fit_in_plan,
"fit-text has no issue; must not appear in plan"
);
}
/// Load the `stacked-horizontal-padding` fixture and assert equivalence.
///
/// This fixture exercises an `Info`-severity `StackedHorizontalPadding`
/// issue: the entire plan should be empty (detect_and_fix applies zero
/// fixes) and both docs remain unchanged.
#[test]
fn equivalence_stacked_horizontal_padding() {
let raw = include_str!("../tests/fixtures/docs/stacked-horizontal-padding.json");
let doc_a: PenDocument = serde_json::from_str(raw).expect("parse");
let doc_b: PenDocument = serde_json::from_str(raw).expect("parse");
let mut clone_a = doc_a;
let report = detect_and_fix(&mut clone_a);
let mut clone_b = doc_b;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
// All issues are Info severity — no fix is applied.
assert_eq!(report.total, 0, "detect_and_fix must apply zero fixes");
assert_eq!(plan.len(), 0, "plan must be empty for all-Info fixtures");
assert_eq!(applied, 0);
assert_eq!(clone_a, clone_b);
}
/// Smoke-test: a `Remove`-action fix (empty-path fixture).
#[test]
fn equivalence_empty_path_remove() {
let d = doc(serde_json::json!({
"version": "1.0",
"children": [{"type": "frame", "id": "root", "children": [
{"type": "path", "id": "empty"}
]}]
}));
let mut clone_a = d.clone();
let report = detect_and_fix(&mut clone_a);
let mut clone_b = d;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(report.total as usize, applied);
assert_eq!(clone_a, clone_b);
}
/// The `status-bar` role guard: a `Remove`-action fix on a status-bar node
/// must NOT appear in the plan (mirrors the guard in `apply_remove`).
#[test]
fn equivalence_remove_status_bar_skipped() {
let d = doc(serde_json::json!({
"version": "1.0",
"children": [{"type": "frame", "id": "root", "children": [
{"type": "frame", "id": "bar", "role": "status-bar"}
]}]
}));
let mut clone_a = d.clone();
let report = detect_and_fix(&mut clone_a);
let mut clone_b = d;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(report.total, 0, "status-bar must not be removed");
assert_eq!(plan.len(), 0, "status-bar must not appear in the plan");
assert_eq!(applied, 0);
assert_eq!(clone_a, clone_b);
}
/// The `UnexpectedRotation` fix (Rotation property).
#[test]
fn equivalence_unexpected_rotation() {
let raw = include_str!("../tests/fixtures/docs/unexpected-rotation.json");
let doc_a: PenDocument = serde_json::from_str(raw).expect("parse");
let doc_b: PenDocument = serde_json::from_str(raw).expect("parse");
let mut clone_a = doc_a;
let report = detect_and_fix(&mut clone_a);
let mut clone_b = doc_b;
let plan = detect_and_plan(&clone_b);
let applied: usize = plan
.iter()
.filter(|fix| apply_planned_fix_to_doc(&mut clone_b, fix))
.count();
assert_eq!(report.total as usize, applied);
assert_eq!(clone_a, clone_b);
}
}

23
crates/op-design-lint/tests/fixtures/docs/invisible-container-with-var.json vendored Normal file
View file

@ -0,0 +1,23 @@
{
"version": "1.0",
"variables": {
"color-border": { "type": "color", "value": "#CBD5E1" }
},
"children": [
{
"type": "frame",
"id": "root",
"layout": "vertical",
"fill": [{ "type": "solid", "color": "#FFFFFF" }],
"children": [
{
"type": "frame",
"id": "light-on-light",
"layout": "vertical",
"fill": [{ "type": "solid", "color": "#FFFFFF" }],
"children": [{ "type": "text", "id": "lol-label", "content": "Inside light wrapper" }]
}
]
}
]
}

19
crates/op-design-lint/tests/fixtures/golden/invisible-container-with-var.json vendored Normal file
View file

@ -0,0 +1,19 @@
[
{
"nodeId": "light-on-light",
"category": "invisible-container",
"severity": "warning",
"property": "stroke",
"currentValue": null,
"suggestedValue": {
"thickness": 1,
"fill": [
{
"type": "solid",
"color": "$color-border"
}
]
},
"reason": "same fill as parent (#FFFFFF ≈ #FFFFFF, contrast=1.00)"
}
]

4
crates/op-design-lint/tests/parity.rs
View file

@ -178,6 +178,10 @@ parity_case!(parity_edge_section_padding, "edge-section-padding");
parity_case!(parity_empty_path, "empty-path");
parity_case!(parity_excessive_frame_effects, "excessive-frame-effects");
parity_case!(parity_invisible_container, "invisible-container");
parity_case!(
parity_invisible_container_with_var,
"invisible-container-with-var"
);
parity_case!(parity_mixed_sibling, "mixed-sibling");
parity_case!(parity_sibling_inconsistency, "sibling-inconsistency");
parity_case!(

23
crates/op-editor-core/src/command_node_attrs.rs
View file

@ -171,8 +171,19 @@ impl EditorState {
/// `SetNodeStrokeHex` — set the stroke color on a node. A node with
/// no stroke gets a fresh 1-px stroke so the color always lands.
///
/// Accepts either a literal `#RGB`/`#RRGGBB` hex OR a `$variable-name`
/// design-token reference. The underlying `PenStroke.fill[*].color`
/// field is a free `String`; `op-design-lint` detectors emit
/// `$color-border` refs when the doc declares that variable, and a
/// strict hex parse here would silently drop them (regression caught
/// by a stop-time review of `LintPreValidator`).
pub(crate) fn cmd_set_node_stroke_hex(&mut self, node_id: &NodeId, hex: &str) -> bool {
if !node_id.is_real() || crate::color_picker::parse_hex_rgb(hex).is_none() {
if !node_id.is_real() {
return false;
}
let is_var_ref = hex.starts_with('$');
if !is_var_ref && crate::color_picker::parse_hex_rgb(hex).is_none() {
return false;
}
let Some(node) = find_node_mut(self.active_children_mut(), node_id) else {
@ -216,8 +227,16 @@ impl EditorState {
}
/// `SetNodeFillHex` — set the fill color on a node by id.
///
/// Accepts either a literal `#RGB`/`#RRGGBB` hex OR a `$variable-name`
/// design-token reference (same rationale as `SetNodeStrokeHex` —
/// `PenFill::Solid.color` is a free `String`).
pub(crate) fn cmd_set_node_fill_hex(&mut self, node_id: &NodeId, hex: &str) -> bool {
if !node_id.is_real() || crate::color_picker::parse_hex_rgb(hex).is_none() {
if !node_id.is_real() {
return false;
}
let is_var_ref = hex.starts_with('$');
if !is_var_ref && crate::color_picker::parse_hex_rgb(hex).is_none() {
return false;
}
let Some(node) = find_node_mut(self.active_children_mut(), node_id) else {

2
crates/op-host-desktop/Cargo.toml
View file

@ -48,6 +48,8 @@ op-host-native = { path = "../op-host-native", version = "0.1.0" }
# `op_editor_core::EditorState` (canonical `.op` document) instead of the
# old shell-core `Document`.
op-editor-core = { path = "../op-editor-core" }
# D′ architecture: pre-validation adapter translates PlannedFix → EditorCommand.
op-design-lint = { path = "../op-design-lint" }
# Locale tables — git dialog strings go through `op_i18n::translate`
# so they are localized like the rest of the chrome.
op-i18n = { path = "../op-i18n" }

17
crates/op-host-desktop/src/chat_orchestrator.rs
View file

@ -47,8 +47,8 @@ use futures::StreamExt;
use op_editor_core::{EditorCommand, EditorState};
use op_orchestrator::{
AbortFlag, CallRequest, DesignRequest, DocSink, LlmChunk, LlmClient, LlmError, Orchestrator,
OrchestratorError, Progress, RunSummary, SkippedPreValidator, SkippedScreenshotProvider,
SkippedVisionLlmClient, ValidationProviders,
OrchestratorError, Progress, RunSummary, SkippedScreenshotProvider, SkippedVisionLlmClient,
ValidationProviders,
};
/// `LlmClient` 的 desktop 实现。每次 `call` 新建一个 `QueryEngine`
@ -194,13 +194,12 @@ pub async fn run_design_request(
visual_ref_enabled: false,
};
let mut sink = DesktopDocSink::new(state);
// Stub validation providers — production-visible, no-op. Host can
// swap in real `PreValidator` (when `op-design-lint` is wired),
// `ScreenshotProvider` (when `jian-skia::captureRegion` is exposed
// host-side), and `VisionLlmClient` (when a Rust multimodal client
// lands) without touching `op-orchestrator`. Today `validation_enabled
// = false` short-circuits the validation phase anyway.
let pre_validator = SkippedPreValidator;
// PreValidator: real `op-design-lint` adapter (D′ architecture, task #35).
// `ScreenshotProvider` and `VisionLlmClient` remain stubbed until
// `jian-skia::captureRegion` is exposed host-side and a Rust multimodal
// client lands. Today `validation_enabled = false` short-circuits the
// vision loop; only `run_pre_validation_fixes` is exercised per request.
let pre_validator = crate::pre_validator::LintPreValidator;
let screenshot = SkippedScreenshotProvider;
let vision = SkippedVisionLlmClient;
let providers = ValidationProviders {

1
crates/op-host-desktop/src/main.rs
View file

@ -29,6 +29,7 @@ mod menu;
mod model_discovery;
mod persistence;
mod persistence_image;
mod pre_validator;
mod settings_io;
mod update_check;
mod window_state;

420
crates/op-host-desktop/src/pre_validator.rs Normal file
View file

@ -0,0 +1,420 @@
//! D′ architecture host adapter — translates `PlannedFix → EditorCommand`.
//!
//! `LintPreValidator` implements `op_orchestrator::PreValidator` by:
//!
//! 1. Cloning the current document from `sink.state().doc`.
//! 2. Calling `op_design_lint::detect_and_plan` to get a `Vec<PlannedFix>`.
//! 3. Translating each `PlannedFix` to one or more `EditorCommand`s via
//! [`planned_fix_to_commands`].
//! 4. Applying each command via `sink.apply` and counting successes.
//!
//! ## FixProperty → EditorCommand translation table
//!
//! | `PlannedAction` | `EditorCommand`(s) |
//! |------------------------|----------------------------------------------------------|
//! | `Remove` | `DeleteNode { node_id }` |
//! | `SetHeightFitContent` | `SetNodeLayoutProp { property:"height", Keyword("fit_content") }` |
//! | `SetRotation(deg)` | `SetNodeRotation { degrees }` |
//! | `SetCornerRadius(r)` | `SetNodeCornerRadius { radius }` |
//! | `SetFontSize(px)` | `SetNodeFontSize { font_size }` |
//! | `ClearEffects` | N × `RemoveNodeEffect { index }` (highest→lowest) |
//! | `SetPadding(v)` | `SetNodeLayoutProp { property:"padding", NumberArray/Number }` |
//! | `SetStroke(v)` | `SetNodeStrokeHex { hex }` + `SetNodeStrokeWidth { width }` |
//!
//! ### `ClearEffects`
//! `EditorCommand` has no "clear all effects" variant — the adapter reads the
//! current effect count from `sink.state()` and emits `RemoveNodeEffect` for
//! each index, iterating highest-to-lowest so index shifts don't affect earlier
//! removals. After all removals the effects field is `None`, matching
//! `op_design_lint::node_mut::clear_effects`.
//!
//! ### `SetStroke`
//! `EditorCommand` has no "set full PenStroke object" variant. The adapter
//! decomposes the JSON object into:
//! - `SetNodeStrokeHex { hex }` — extracts the first solid-fill color from
//! the suggested value's `fill[0].color`; creates a fresh 1px stroke if
//! the node has none.
//! - `SetNodeStrokeWidth { width }` — extracts `thickness` as `f32`.
//!
//! The resulting `PenStroke` is structurally identical to what
//! `node_mut::set_stroke_from_json` produces for the current fixture set
//! (all invisible-container stroke fixes use `{thickness:1, fill:[{type:solid,
//! color:"#E2E8F0"}]}`). This is verified by the parity integration test.
//! If a future detector emits a stroke with non-solid fills or per-side
//! thickness, extend the translation or report a gap.
use op_design_lint::plan::{PlannedAction, PlannedFix};
use op_editor_core::{EditorCommand, EditorState};
use op_orchestrator::{DocSink, PreValidationResult, PreValidator};
/// Real `PreValidator` implementation backed by `op-design-lint`.
///
/// Swap `SkippedPreValidator` for this in `chat_orchestrator.rs` to
/// enable pre-validation fixes on every design generation run.
pub struct LintPreValidator;
impl PreValidator for LintPreValidator {
fn run_pre_validation_fixes(&self, sink: &mut dyn DocSink) -> PreValidationResult {
// Clone the document snapshot so detect_and_plan can read it
// immutably while sink remains available for apply calls.
let doc_clone = sink.state().doc.clone();
let plan = op_design_lint::detect_and_plan(&doc_clone);
let mut total = 0usize;
let mut by_category: std::collections::BTreeMap<String, usize> = Default::default();
for fix in &plan {
let commands = planned_fix_to_commands(fix, sink.state());
// Count this fix as applied only if at least one command lands.
let mut any_applied = false;
for cmd in commands {
if sink.apply(cmd) {
any_applied = true;
}
}
if any_applied {
total += 1;
let key = serde_json::to_value(fix.category)
.ok()
.and_then(|v| v.as_str().map(str::to_string))
.unwrap_or_else(|| format!("{:?}", fix.category));
*by_category.entry(key).or_insert(0) += 1;
}
}
PreValidationResult { total, by_category }
}
}
/// Translate one `PlannedFix` into the `EditorCommand`(s) needed to apply it.
/// Returns an empty vec only for configurations where no command is appropriate
/// (e.g. a `SetStroke` whose JSON has no extractable hex — should not happen
/// in practice since `detect_and_plan` only emits valid suggested values).
fn planned_fix_to_commands(fix: &PlannedFix, state: &EditorState) -> Vec<EditorCommand> {
use op_editor_core::node_id::NodeId;
let node_id = NodeId::new(&*fix.node_id);
let mut cmds = Vec::new();
match &fix.action {
PlannedAction::Remove => {
cmds.push(EditorCommand::DeleteNode {
node_id: node_id.clone(),
});
}
PlannedAction::SetHeightFitContent => {
cmds.push(EditorCommand::SetNodeLayoutProp {
node_id: node_id.clone(),
property: "height".to_string(),
value: op_editor_core::LayoutPropValue::Keyword("fit_content".to_string()),
});
}
PlannedAction::SetRotation(deg) => {
cmds.push(EditorCommand::SetNodeRotation {
node_id: node_id.clone(),
degrees: *deg as f32,
});
}
PlannedAction::SetCornerRadius(r) => {
cmds.push(EditorCommand::SetNodeCornerRadius {
node_id: node_id.clone(),
radius: *r,
});
}
PlannedAction::SetFontSize(px) => {
cmds.push(EditorCommand::SetNodeFontSize {
node_id: node_id.clone(),
font_size: *px,
});
}
PlannedAction::ClearEffects => {
// Read the current effect count from the state, then emit
// RemoveNodeEffect for each index from highest to lowest so
// earlier indices are not invalidated by the removal.
let effect_count = effect_count_for_node(state, &node_id);
for i in (0..effect_count).rev() {
cmds.push(EditorCommand::RemoveNodeEffect {
node_id: node_id.clone(),
index: i as u32,
});
}
}
PlannedAction::SetPadding(v) => {
// Translate to SetNodeLayoutProp "padding" with Number or NumberArray.
let prop_value = if let Some(n) = v.as_f64() {
Some(op_editor_core::LayoutPropValue::Number(n))
} else if let Some(arr) = v.as_array() {
let nums: Option<Vec<f64>> = arr.iter().map(|x| x.as_f64()).collect();
nums.map(op_editor_core::LayoutPropValue::NumberArray)
} else {
None
};
if let Some(pv) = prop_value {
cmds.push(EditorCommand::SetNodeLayoutProp {
node_id: node_id.clone(),
property: "padding".to_string(),
value: pv,
});
}
}
PlannedAction::SetStroke(v) => {
// Decompose the PenStroke JSON into SetNodeStrokeHex + SetNodeStrokeWidth.
// Both commands are emitted; the hex command creates a fresh stroke if needed.
//
// Extract hex from fill[0].color (first solid fill entry).
let hex = v
.get("fill")
.and_then(|f| f.as_array())
.and_then(|arr| arr.first())
.and_then(|entry| entry.get("color"))
.and_then(|c| c.as_str())
.map(str::to_string);
// Extract thickness as f32.
let thickness = v
.get("thickness")
.and_then(|t| t.as_f64())
.map(|t| t as f32);
if let Some(hex) = hex {
cmds.push(EditorCommand::SetNodeStrokeHex {
node_id: node_id.clone(),
hex,
});
}
if let Some(width) = thickness {
cmds.push(EditorCommand::SetNodeStrokeWidth {
node_id: node_id.clone(),
width,
});
}
}
}
cmds
}
/// Count the number of effects on the named node in the active page.
/// Returns 0 if the node is not found or has no effects.
fn effect_count_for_node(state: &EditorState, node_id: &op_editor_core::node_id::NodeId) -> usize {
use op_editor_core::walkers::find_node;
let children = state.active_children();
let Some(node) = find_node(children, node_id) else {
return 0;
};
effects_from_pen_node(node).len()
}
/// Extract the effect list from a `PenNode`, returning an empty slice
/// when the node kind has no effects or effects is `None`.
fn effects_from_pen_node(
node: &jian_ops_schema::node::PenNode,
) -> &[jian_ops_schema::style::PenEffect] {
use jian_ops_schema::node::PenNode;
let opt = match node {
PenNode::Frame(n) => n.container.effects.as_deref(),
PenNode::Group(n) => n.container.effects.as_deref(),
PenNode::Rectangle(n) => n.container.effects.as_deref(),
PenNode::Ellipse(n) => n.effects.as_deref(),
PenNode::Polygon(n) => n.effects.as_deref(),
PenNode::Path(n) => n.effects.as_deref(),
PenNode::Line(n) => n.effects.as_deref(),
PenNode::Text(n) => n.effects.as_deref(),
PenNode::TextInput(n) => n.effects.as_deref(),
PenNode::Image(n) => n.effects.as_deref(),
PenNode::IconFont(_) | PenNode::Ref(_) => None,
};
opt.unwrap_or(&[])
}
// ── Parity integration tests ─────────────────────────────────────────────────
//
// For each of the same 3 fixtures used in `op-design-lint`'s equivalence tests:
// - Build an `EditorState` whose `doc` is the fixture's `PenDocument`.
// - Wrap it in a `TestSink`.
// - Call `LintPreValidator.run_pre_validation_fixes(&mut sink)`.
// - Compare `sink.state().doc` against the result of
// `op_design_lint::detect_and_fix(&mut fixture_doc.clone())`.
// - Assert both produce structurally-identical `PenDocument`s.
//
// This is the LOAD-BEARING test that proves the cross-crate adapter doesn't
// silently lose mutations.
#[cfg(test)]
mod tests {
use super::*;
use op_editor_core::EditorState;
use op_orchestrator::PreValidator;
/// Minimal `DocSink` for tests — wraps an `EditorState`, applies
/// commands through `EditorState::apply`. Modeled on
/// `op-orchestrator::test_support::VecDocSink`.
struct TestSink {
state: EditorState,
}
impl TestSink {
fn from_doc(doc: jian_ops_schema::PenDocument) -> Self {
Self {
state: EditorState::from_document(doc),
}
}
}
impl op_orchestrator::DocSink for TestSink {
fn state(&self) -> &EditorState {
&self.state
}
fn apply(&mut self, cmd: op_editor_core::EditorCommand) -> bool {
self.state.apply(cmd)
}
fn begin_undo_batch(&mut self) {}
fn end_undo_batch(&mut self) {}
}
fn load_doc(json: &str) -> jian_ops_schema::PenDocument {
serde_json::from_str(json).expect("fixture must parse as PenDocument")
}
/// Normalize a `serde_json::Value` for structural comparison.
///
/// `EditorState::find_node_mut` calls `children_mut()` which uses
/// `get_or_insert_with(Vec::new)` — this side-effects frames that
/// were traversed but not mutated from `children: None` →
/// `children: Some([])`. The `detect_and_fix` path does NOT do
/// this. We normalize away this difference by removing any JSON
/// `"children"` key whose value is an empty array (`[]`).
///
/// Every other field is compared exactly. This is the only known
/// structural difference between the two paths.
fn normalize_doc_json(v: serde_json::Value) -> serde_json::Value {
use serde_json::Value;
match v {
Value::Object(mut map) => {
// Remove "children": [] (empty array) — artifact of
// EditorState walker's get_or_insert_with(Vec::new).
if let Some(Value::Array(arr)) = map.get("children") {
if arr.is_empty() {
map.remove("children");
}
}
Value::Object(
map.into_iter()
.map(|(k, v)| (k, normalize_doc_json(v)))
.collect(),
)
}
Value::Array(arr) => Value::Array(arr.into_iter().map(normalize_doc_json).collect()),
other => other,
}
}
/// Assert that `LintPreValidator` + `EditorState` produces the same
/// `PenDocument` as `op_design_lint::detect_and_fix` on the same input.
///
/// Structural comparison uses JSON normalization to handle the known
/// `children: None` vs `children: Some([])` difference introduced by
/// `EditorState::find_node_mut`'s `get_or_insert_with(Vec::new)` side
/// effect (see `normalize_doc_json`).
fn assert_parity(fixture_json: &str, label: &str) {
// Reference path: detect_and_fix on a raw clone.
let mut reference_doc = load_doc(fixture_json);
let reference_report = op_design_lint::detect_and_fix(&mut reference_doc);
// Adapter path: LintPreValidator through EditorState.
let adapter_doc = load_doc(fixture_json);
let mut sink = TestSink::from_doc(adapter_doc);
let result = LintPreValidator.run_pre_validation_fixes(&mut sink);
assert_eq!(
reference_report.total as usize, result.total,
"{label}: applied counts must match (reference={}, adapter={})",
reference_report.total, result.total
);
// Compare via normalized JSON to handle children: None vs Some([]).
let ref_json = serde_json::to_value(&reference_doc).expect("serialize reference");
let adapter_json = serde_json::to_value(&sink.state().doc).expect("serialize adapter");
let ref_norm = normalize_doc_json(ref_json);
let adapter_norm = normalize_doc_json(adapter_json);
assert_eq!(
ref_norm, adapter_norm,
"{label}: documents must be structurally identical after both fix paths"
);
}
/// invisible-container: exercises `FixProperty::Stroke` (Warning)
/// and verifies Info-severity dark-on-dark is NOT applied.
#[test]
fn parity_invisible_container() {
assert_parity(
include_str!("../../op-design-lint/tests/fixtures/docs/invisible-container.json"),
"invisible-container",
);
}
/// text-explicit-height: exercises `FixProperty::Height` → `SetHeightFitContent`.
#[test]
fn parity_text_explicit_height() {
assert_parity(
include_str!("../../op-design-lint/tests/fixtures/docs/text-explicit-height.json"),
"text-explicit-height",
);
}
/// stacked-horizontal-padding: all Info severity — zero fixes applied;
/// doc remains unchanged under both paths.
#[test]
fn parity_stacked_horizontal_padding() {
assert_parity(
include_str!(
"../../op-design-lint/tests/fixtures/docs/stacked-horizontal-padding.json"
),
"stacked-horizontal-padding",
);
}
/// unexpected-rotation: exercises `FixProperty::Rotation` → `SetNodeRotation`.
#[test]
fn parity_unexpected_rotation() {
assert_parity(
include_str!("../../op-design-lint/tests/fixtures/docs/unexpected-rotation.json"),
"unexpected-rotation",
);
}
/// empty-path: exercises `FixProperty::Remove` → `DeleteNode`.
#[test]
fn parity_empty_path() {
assert_parity(
include_str!("../../op-design-lint/tests/fixtures/docs/empty-path.json"),
"empty-path",
);
}
/// invisible-container-with-var: exercises `$color-border` design-token
/// reference path through `SetNodeStrokeHex`. Regression guard caught
/// by stop-time review — `cmd_set_node_stroke_hex` previously rejected
/// `$`-prefixed strings via `parse_hex_rgb`, dropping the color silently
/// while `SetNodeStrokeWidth` still applied (so `any_applied=true` and
/// `applied_count` overreported). The op-editor-core fix relaxed both
/// `cmd_set_node_stroke_hex` + `cmd_set_node_fill_hex` to passthrough
/// `$ref` strings (the underlying `color: String` field accepts them).
#[test]
fn parity_invisible_container_with_var() {
assert_parity(
include_str!(
"../../op-design-lint/tests/fixtures/docs/invisible-container-with-var.json"
),
"invisible-container-with-var",
);
}
}