diff --git a/crates/op-ai-skills/skills/phases/agent/design-agent.md b/crates/op-ai-skills/skills/phases/agent/design-agent.md index e81ae1aef..b387c2728 100644 --- a/crates/op-ai-skills/skills/phases/agent/design-agent.md +++ b/crates/op-ai-skills/skills/phases/agent/design-agent.md @@ -222,6 +222,28 @@ When the canvas already holds one or more screens, a new screen (a profile page 4. **Same frame contract.** The new screen root uses the SAME width/height class as its siblings and opens to the RIGHT (`find_empty_space`). 5. **Different content, same skeleton vocabulary.** Vary the content architecture to the screen's purpose, but compose it from the same component vocabulary (same card style, same list-row anatomy, same button hierarchy) the first screen established. +## Multi-Screen Interactivity (App Mode Preview) + +When the canvas holds **2 or more screens**, the preview engine can turn them into a tappable, navigable app if — and only if — you mark them. This is optional groundwork you do yourself; nothing else in the tool loop requires it, but skipping it means Preview stays a flat scrolling page instead of switching screens on tap. + +1. **Mark every top-level screen frame with `screen`.** Exactly ONE screen is the entry, marked `"screen": "/"`; every other screen gets a unique `/slug` (e.g. `"/profile"`, `"/settings"`) — unique across the WHOLE document. Set it directly on the frame node, e.g. `I(null, {type:"frame", name:"Profile", screen:"/profile", ...})` or `U(profileFrameId, {screen:"/profile"})`. +2. **Bind every tappable navigation element to `events.onTap`.** A bottom-tab-bar item, a sidebar-nav row, or a card that opens a detail screen all bind the same way — a wire-format action object whose body is the JSON STRING `"\"/path\""` (the quote characters are literally part of the string value; this compiles as a Tier-1 expression, a bare `/path` does not): + + ```json + { "events": { "onTap": [ { "replace": "\"/profile\"" } ] } } + ``` + ```json + { "events": { "onTap": [ { "push": "\"/detail/42\"" } ] } } + ``` + + Use `replace` for tab-bar / sidebar switches (lateral navigation between sibling screens); use `push` for drilling into a detail view a user expects to come back FROM. A header/back-arrow control binds `{"pop": null}` (no path): + + ```json + { "events": { "onTap": [ { "pop": null } ] } } + ``` +3. **Never write a `route` field.** `route` is schema-only surface metadata the preview engine's tap dispatcher does not consume — only `events.onTap` drives navigation. Writing `route` instead of `events.onTap` looks plausible but does nothing when tapped. +4. If a `batch_design` result carries `navIssues`, it is naming nav-tab items that already sit on a screen you marked but are not yet bound — bind each one exactly as shown in the issue; do not guess a different destination. + ## Parallel Work — `spawn_agents` For a large multi-screen task (more than 3–4 screens): diff --git a/crates/op-ai-skills/skills/phases/generation/interactivity.md b/crates/op-ai-skills/skills/phases/generation/interactivity.md index c83927d1f..d727dc606 100644 --- a/crates/op-ai-skills/skills/phases/generation/interactivity.md +++ b/crates/op-ai-skills/skills/phases/generation/interactivity.md @@ -72,13 +72,17 @@ Supported event hook keys (camelCase, `#[serde(rename_all = "camelCase")]`): Action vocabulary (body shape per action): -| Action | Body | Effect | -|----------|------------------------------------------------------------------|------------------------------------------| -| `set` | `{ "": "" }` map of assignments | Write one or more state variables | -| `toggle` | `""` — the bool variable to flip | Toggle a bool state variable | -| `toast` | `""` string or template literal | Show a transient notification | -| `push` | `""` string | Navigate to a named route | -| `if` | `{ "expr": "", "then": [...], "else": [...] }` | Conditional action branch (`else` optional) | +| Action | Body | Effect | +|-----------|------------------------------------------------------------------|------------------------------------------| +| `set` | `{ "": "" }` map of assignments | Write one or more state variables | +| `toggle` | `""` — the bool variable to flip | Toggle a bool state variable | +| `toast` | `""` string or template literal | Show a transient notification | +| `push` | `"\"\""` — a JSON string whose VALUE is itself `""`, quotes included | Drill into a screen (keeps the caller reachable via back/pop) | +| `replace` | `"\"\""` — same quote-literal shape as `push` | Switch to a sibling screen (tab bar / sidebar — no back entry) | +| `pop` | `null` — no body | Return to the previous screen | +| `if` | `{ "expr": "", "then": [...], "else": [...] }` | Conditional action branch (`else` optional) | + +`push` / `replace` bodies compile as a Tier-1 EXPRESSION, not a literal path — an unquoted `/stats` lexes as a division token and fails to compile. Always wrap the route path in an extra pair of escaped quotes: `{ "push": "\"/stats\"" }`, never `{ "push": "/stats" }`. Examples (grounded in `full-jian-extensions.op` + `form.op`): @@ -89,7 +93,7 @@ Examples (grounded in `full-jian-extensions.op` + `form.op`): { "if": { "expr": "$app.count >= $app.target", - "then": [{ "toast": "Done!" }, { "push": "/stats" }] + "then": [{ "toast": "Done!" }, { "push": "\"/stats\"" }] } } ] @@ -115,6 +119,37 @@ EXPRESSION LANGUAGE: - Template literals (backtick, resolved by the expression parser): `` `Count: ${$app.count}` `` +MULTI-SCREEN NAVIGATION (App Mode preview) — `screen` marker + tap-to-switch: + +A document with 2+ screens becomes a tappable, navigable app in Preview once +its top-level screen frames carry a `screen` route path and its nav elements +bind `push` / `replace` / `pop` as above. Mark exactly ONE top-level frame +`"screen": "/"` (the entry); every other screen gets a unique `/slug`, unique +across the whole document: + +```json +{ "type": "frame", "id": "home", "name": "Home", "screen": "/" } +{ "type": "frame", "id": "profile", "name": "Profile", "screen": "/profile" } +``` + +Bind a bottom-tab-bar / sidebar item with `replace` (lateral move between +sibling screens); bind a card/row that opens a detail screen with `push` +(the user expects to come back FROM it); bind a header back-arrow with `pop`: + +```json +{ "events": { "onTap": [ { "replace": "\"/profile\"" } ] } } +``` + +```json +{ "events": { "onTap": [ { "pop": null } ] } } +``` + +`screen` is valid ONLY on a top-level frame — a nested frame's `screen` value +is ignored by the routing projection. Never write a `route` field instead of +`events.onTap`: `route` is schema-only surface metadata the tap dispatcher +does not read, so a node with `route` but no `events.onTap` does nothing +when tapped. + PLACEMENT RULES: - Declare `state` on the **lowest common ancestor** node that all bindings / @@ -138,6 +173,10 @@ CORRECTNESS CHECKLIST: separate array elements). - `set` body is an object mapping variable paths to expression strings. - `toggle` body is a single string (the variable path), not an object. -- `push` body is a route path string, not an object. +- `push` / `replace` bodies are the quote-literal string `"\"\""`, not a + bare path string and not an object — a bare `/path` fails to compile. +- `pop` body is `null`, never a path. - `if` body has an `expr` string plus `then` array; `else` is optional. - Expression strings are plain JSON strings — no special encoding needed. +- `screen` is a plain string on a top-level frame, never `route` — `route` + is not consumed by the tap dispatcher. diff --git a/crates/op-ai-skills/skills/phases/generation/schema.md b/crates/op-ai-skills/skills/phases/generation/schema.md index 447506815..828696fbf 100644 --- a/crates/op-ai-skills/skills/phases/generation/schema.md +++ b/crates/op-ai-skills/skills/phases/generation/schema.md @@ -18,6 +18,7 @@ PenNode types (the ONLY format you output for designs): - image: Props: width, height, cornerRadius, effects, imageSearchQuery (2-3 English keywords UNIQUE per image — derive from the surrounding card/dish/title text; reusing one query across multiple images makes every card render the same photo. For food cards, use prepared-dish queries like "pasta plate", "salmon bowl", "pizza plate", "sushi platter"; avoid ingredient-only, outdoor/grass, raw-object, or novelty queries), imagePrompt (a fuller natural-language description of the SAME subject for AI image generation — e.g. "professional food photography of a pasta plate, warm natural light, shallow depth of field". ALWAYS emit it alongside imageSearchQuery: a configured image-gen model uses imagePrompt for a rich original image, otherwise imageSearchQuery drives the stock-search fallback — so every image element carries both) All nodes share: id, type, name, role, x, y, rotation, opacity +Interactivity (multi-screen apps only): frame accepts `screen` (a top-level frame's route path — `"/"` for the entry screen, `"/slug"` for every other screen, unique across the document) and `events` (`{ onTap: [ {"replace": "\"/path\""} | {"push": "\"/path\""} | {"pop": null} ] }` — the action body is the literal JSON STRING `"\"/path\""`, quote characters included, since it compiles as an expression; a bare `/path` fails). Never write `route` — it is schema-only metadata the tap dispatcher ignores. Fill = [{ type: "solid", color: "#hex" }] or [{ type: "linear_gradient", angle, stops: [{ offset, color }] }] or [{ type: "radial_gradient", stops: [{ offset, color }] }] or [{ type: "mesh_gradient", rows, cols, stops: [{ row, col, color }] }] - radial_gradient example (concentric glow): `fill: [{ type: "radial_gradient", cx: 0.5, cy: 0.5, radius: 0.7, stops: [{ offset: 0, color: "#6d28d9" }, { offset: 1, color: "#0b0614" }] }]` — cx/cy are 0..1 fractions of the box (0.5 = centre); radius is a 0..1 fraction of max(w,h). - mesh_gradient example (smooth four-corner blend): `fill: [{ type: "mesh_gradient", rows: 2, cols: 2, stops: [{ row: 0, col: 0, color: "#ec4899" }, { row: 0, col: 1, color: "#8b5cf6" }, { row: 1, col: 0, color: "#3b82f6" }, { row: 1, col: 1, color: "#06b6d4" }] }]` — a rows×cols vertex grid Gouraud-interpolated across the box; each stop pins one vertex colour at (row, col). Use it for rich multi-hue hero/background panels instead of stacking layers. diff --git a/crates/op-ai-skills/src/lib.rs b/crates/op-ai-skills/src/lib.rs index 1cc3823fd..55c039feb 100644 --- a/crates/op-ai-skills/src/lib.rs +++ b/crates/op-ai-skills/src/lib.rs @@ -56,13 +56,63 @@ pub fn append_image_self_check_scope(prompt: &mut String) { prompt.push_str(&block); } +/// Single source of truth for `guideline_for`'s dispatch AND for every +/// caller (e.g. `op-mcp`'s `get_guidelines` unknown-topic error) that wants +/// to name the full supported-topic set without hardcoding a second, +/// driftable copy. Each row is `(primary_name, aliases, skill_names)`: the +/// primary name plus its aliases all resolve to the same composed guideline, +/// built by concatenating `skill_names` in order (see [`compose_skills`]). +/// +/// Adding a topic is a ONE-LINE change here — `guideline_for` and +/// [`guideline_topics`] both read this table, so neither can go stale +/// relative to the other. +const GUIDELINE_TOPICS: &[(&str, &[&str], &[&str])] = &[ + ( + "web-app", + &["webapp"], + &["product-principles", "web-app", "design-principles"], + ), + ("mobile", &["mobile-app"], &["mobile-app"]), + ("code-to-design", &[], &["code-to-design"]), + ( + "landing-page", + &["landing"], + &["landing-page", "design-principles"], + ), + ( + "dashboard", + &["table"], + &["dashboard", "product-principles"], + ), + ("slides", &["deck", "presentation"], &["slides"]), + ("form", &["form-ui"], &["form-ui"]), + ("design-system", &[], &["design-system"]), + ("interactivity", &[], &["interactivity"]), +]; + +/// Compose the named skills (in order) into one coherent guideline doc, +/// skipping any that are absent. `None` if nothing resolved. +fn compose_skills(names: &[&str]) -> Option { + let parts: Vec<&str> = names + .iter() + .filter_map(|&n| get_skill_by_name(n).map(|s| s.content.trim())) + .filter(|c| !c.is_empty()) + .collect(); + if parts.is_empty() { + None + } else { + Some(parts.join("\n\n")) + } +} + /// Return the product-design guideline text for `topic`, composed from the /// embedded skill corpus. Mirrors Pencil's `get_guidelines(guide, …)`: each /// topic resolves to its focused task guide plus the principle skills that /// complete it. All content is local (embedded via `include_dir!`) — there is /// no remote fetch. /// -/// Supported topics (aliases in parens): +/// Supported topics (aliases in parens) — see [`GUIDELINE_TOPICS`] for the +/// canonical table; [`guideline_topics`] returns the flattened name list: /// - `"web-app"` (`webapp`) — product principles + web-app depth laws + design craft /// - `"mobile"` (`mobile-app`) — the mobile-app three-section architecture /// - `"code-to-design"` — agent workflow for converting frontend codebases @@ -71,35 +121,25 @@ pub fn append_image_self_check_scope(prompt: &mut String) { /// - `"slides"` (`deck`, `presentation`) — slide layout contracts /// - `"form"` (`form-ui`) — form-ui domain /// - `"design-system"` — design-system composition +/// - `"interactivity"` — multi-screen navigation contract (`screen` markers +/// + `events.onTap` actions) for tappable App Mode preview /// /// Returns `None` for any unrecognised topic so callers can produce a typed /// "unknown topic" error without special-casing the string themselves. pub fn guideline_for(topic: &str) -> Option { - // Compose the named skills (in order) into one coherent guideline doc, - // skipping any that are absent. `None` if nothing resolved. - fn compose(names: &[&str]) -> Option { - let parts: Vec<&str> = names - .iter() - .filter_map(|&n| get_skill_by_name(n).map(|s| s.content.trim())) - .filter(|c| !c.is_empty()) - .collect(); - if parts.is_empty() { - None - } else { - Some(parts.join("\n\n")) - } - } - match topic { - "web-app" | "webapp" => compose(&["product-principles", "web-app", "design-principles"]), - "mobile" | "mobile-app" => compose(&["mobile-app"]), - "code-to-design" => compose(&["code-to-design"]), - "landing-page" | "landing" => compose(&["landing-page", "design-principles"]), - "dashboard" | "table" => compose(&["dashboard", "product-principles"]), - "slides" | "deck" | "presentation" => compose(&["slides"]), - "form" | "form-ui" => compose(&["form-ui"]), - "design-system" => compose(&["design-system"]), - _ => None, - } + let (_, _, skill_names) = GUIDELINE_TOPICS + .iter() + .find(|(name, aliases, _)| *name == topic || aliases.contains(&topic))?; + compose_skills(skill_names) +} + +/// The primary name of every topic [`guideline_for`] accepts, in table +/// order — for callers that need to name the full supported-topic set (e.g. +/// an "unknown topic" error hint) without hardcoding a copy that can drift +/// out of sync as topics are added. Aliases are omitted; each is a synonym +/// for the primary name already listed. +pub fn guideline_topics() -> Vec<&'static str> { + GUIDELINE_TOPICS.iter().map(|(name, _, _)| *name).collect() } /// Return the system prompt for the design agentic tool-loop. @@ -381,6 +421,33 @@ mod tests { ); } + #[test] + fn guideline_for_interactivity_teaches_screen_and_on_tap_contract() { + let content = + guideline_for("interactivity").expect("interactivity guideline must be present"); + assert!( + content.contains("\"screen\""), + "must teach the screen marker" + ); + assert!( + content.contains("events.onTap") || content.contains("onTap"), + "must teach the events.onTap binding" + ); + assert!( + content.contains(r#"{ "replace": "\"/profile\"" } "#) + || content.contains(r#"{ "replace": "\"/profile\"" }"#), + "must show the exact quote-literal replace example: {content:?}" + ); + assert!( + content.contains(r#"{"pop": null}"#) || content.contains(r#"{ "pop": null }"#), + "must show the pop (no-path) example" + ); + assert!( + content.contains("`route` field") && content.contains("schema-only"), + "must forbid the schema-only route field: {content:?}" + ); + } + #[test] fn design_agent_system_prompt_resolves_and_contains_protocol_markers() { let prompt = design_agent_system_prompt(); diff --git a/crates/op-host-services/src/design_agent_tools.rs b/crates/op-host-services/src/design_agent_tools.rs index 01445da37..be6dc8450 100644 --- a/crates/op-host-services/src/design_agent_tools.rs +++ b/crates/op-host-services/src/design_agent_tools.rs @@ -193,6 +193,13 @@ pub fn execute_design_tool_with_root_seed_guard( ); } let empty_shells = scan_empty_shells(state.active_children()); + // Track B of the interactive-preview plan: an intent-shaped echo (not + // an auto-fix — see `op_orchestrator::nav_issues` module doc) naming + // any nav-tab item that name-matches an already screen-marked frame + // but has no `events.onTap` bound yet. `wire_screen_navigation` + // (Track A) is the deterministic backstop if the model never gets to + // it before the design ends. + let nav_issues = op_orchestrator::nav_issues::scan_nav_issues(state); let design_diagnostics = crate::design_agent_diagnostics::collect_batch_design_diagnostics(state); layout_issues.extend(design_diagnostics.layout_issues); @@ -207,6 +214,7 @@ pub fn execute_design_tool_with_root_seed_guard( || !intent_questions.is_empty() || !variable_issues.is_empty() || !image_slot_candidates.is_empty() + || !nav_issues.is_empty() || root_seed_hint.is_some() { if let Ok(mut envelope) = serde_json::from_str::(&result.content) { @@ -251,6 +259,9 @@ pub fn execute_design_tool_with_root_seed_guard( ); obj.insert("contrastIssues".into(), serde_json::json!(contrast_issues)); } + if !nav_issues.is_empty() { + obj.insert("navIssues".into(), serde_json::json!(nav_issues)); + } let mut hints = Vec::new(); if !layout_issues.is_empty() { hints.push( @@ -282,6 +293,12 @@ pub fn execute_design_tool_with_root_seed_guard( .to_string(), ); } + if !nav_issues.is_empty() { + hints.push( + "navIssues: this is a multi-screen app - the listed nav tabs are not wired to switch screens yet. Bind each one's events.onTap exactly as shown; do not guess a different destination." + .to_string(), + ); + } if let Some(hint) = root_seed_hint { hints.push(hint); } diff --git a/crates/op-mcp/src/batch_program_interactivity_tests.rs b/crates/op-mcp/src/batch_program_interactivity_tests.rs new file mode 100644 index 000000000..c12ff9cd0 --- /dev/null +++ b/crates/op-mcp/src/batch_program_interactivity_tests.rs @@ -0,0 +1,43 @@ +//! Track B of the interactive-preview plan: `screen` (on `FrameNode`) and +//! `events` (on Frame/Group/Rectangle) are ordinary flattened `PenNode` +//! fields — `parse_node_json`'s `serde_json::from_value::` has no +//! field whitelist, so anything script-gen's compiled `I(parent, json)` +//! line hands it lands on the node unfiltered. This exercises the exact +//! `execute_insert` path a script-gen program compiles down to (see +//! `op_mcp::script_runner::eval_to_program`'s `__record` -> `I(parent, +//! json)` line format) and confirms both fields survive to the inserted +//! node with the model-authored contract's exact wire shape — the +//! quote-literal navigate body (`"\"/detail\""`) included. + +use jian_ops_schema::node::PenNode; + +use super::*; + +#[test] +fn i_call_passes_screen_and_events_through_unfiltered() { + let mut state = sample(); + let program = r##"home=I(null, {"type":"frame","name":"Home","width":390,"height":844,"screen":"/","events":{"onTap":[{"push":"\"/detail\""}]}})"##; + + let (envelope, cmd) = call_operations(&state, program); + assert!(envelope.get("errors").is_none(), "{envelope}"); + let home_id = binding_id(&envelope, "home"); + + assert!(state.apply(cmd.expect("insert must emit a command"))); + let children = state.active_children(); + let home = + op_editor_core::walkers::find_node(children, &NodeId::new(&home_id)).expect("home node"); + let PenNode::Frame(home_frame) = home else { + panic!("expected a frame node, got {home:?}"); + }; + assert_eq!( + home_frame.screen.as_deref(), + Some("/"), + "screen marker must pass through I() unfiltered" + ); + let events_json = serde_json::to_value(home).unwrap()["events"].clone(); + assert_eq!( + events_json, + serde_json::json!({ "onTap": [ { "push": "\"/detail\"" } ] }), + "events.onTap must pass through I() with the exact quote-literal navigate body: {events_json}" + ); +} diff --git a/crates/op-mcp/src/batch_program_tests.rs b/crates/op-mcp/src/batch_program_tests.rs index 06675245c..b6861121b 100644 --- a/crates/op-mcp/src/batch_program_tests.rs +++ b/crates/op-mcp/src/batch_program_tests.rs @@ -591,6 +591,9 @@ D("ghost")"## #[path = "batch_program_image_tests.rs"] mod image_tests; +#[path = "batch_program_interactivity_tests.rs"] +mod interactivity_tests; + #[test] fn post_process_flag_marks_the_envelope() { let state = sample(); diff --git a/crates/op-orchestrator/src/cleanup.rs b/crates/op-orchestrator/src/cleanup.rs index 727405657..f06ca6bbd 100644 --- a/crates/op-orchestrator/src/cleanup.rs +++ b/crates/op-orchestrator/src/cleanup.rs @@ -1449,6 +1449,16 @@ pub fn run_cleanup_passes(sink: &mut dyn DocSink, plan: &OrchestratorPlan, root_ // where that section belongs is intent — the geometry echo handles it. anchor_bottom_nav_last_for_all_roots(sink); crate::mobile_reflow::repair_mobile_trailing_nav_reflow_in_sink(sink); + + // Track A of the interactive-preview plan: mark screen-shaped top-level + // frames + wire their nav tabs / back buttons, so a multi-screen document + // enters App Mode preview with zero model cooperation. Runs LAST — after + // bottom-nav anchoring/dedup/distribution above have settled the final + // nav shape, so tab-item discovery sees the real tree, not an + // in-progress one. Whole-doc (scans `sink.state()`, not `root_ids`) so + // it also links PRE-EXISTING screens from earlier turns, matching + // `avatar_repair` above. + crate::wire_screen_navigation::wire_screen_navigation(sink); } /// Apply a whole-root transform (the serialize → mutate → deserialize round-trip diff --git a/crates/op-orchestrator/src/lib.rs b/crates/op-orchestrator/src/lib.rs index a81a06770..8638fd1c7 100644 --- a/crates/op-orchestrator/src/lib.rs +++ b/crates/op-orchestrator/src/lib.rs @@ -57,6 +57,7 @@ pub(crate) mod cleanup_typography; pub mod concurrent; pub mod geometry_validation; pub mod loop_finalize; +pub mod nav_issues; pub mod prompt; pub mod radial_repair; pub mod role_defaults; @@ -72,6 +73,7 @@ pub mod stub_repair; pub mod subagent; pub mod table_repair; pub mod tree_heuristics; +pub mod wire_screen_navigation; #[cfg(test)] mod cleanup_mobile_chrome_nav_wrapper_tests; diff --git a/crates/op-orchestrator/src/nav_issues.rs b/crates/op-orchestrator/src/nav_issues.rs new file mode 100644 index 000000000..7bbb3d65b --- /dev/null +++ b/crates/op-orchestrator/src/nav_issues.rs @@ -0,0 +1,116 @@ +//! Track B of the interactive-preview plan — the `navIssues` per-batch echo +//! (see +//! `openpencil-docs/openpencil/generation/preview-interactive-app-mode-0712.md`, +//! Track B item 2). `wire_screen_navigation` (Track A) is the deterministic +//! backstop that fills unbound nav tabs once the loop finalizes; this scan +//! runs IN-LOOP, right after a `batch_design` write, so the model can see and +//! fix its own unbound navigation while the design is still open instead of +//! only being caught by Track A's end-of-run repair. +//! +//! Detection: once a document has **≥ 2 top-level frames the model itself +//! already marked with `screen`**, any bottom-tab-bar / sidebar-nav item +//! whose label name-matches one of those screens but carries no `events` yet +//! is flagged, naming the tab's node id and the exact `events.onTap` patch it +//! should bind. This is an ECHO ONLY — it never mutates the document. Which +//! tab a model intends to route where is INTENT, not a structural defect +//! (same "回波不硬修" discipline as the other intent-shaped echoes in +//! `design_agent_tools.rs`), and Track A repairs it anyway if the model never +//! gets to it. +//! +//! Reuses `wire_screen_navigation`'s own nav-container / label-matching / +//! events-presence helpers (`pub(crate)`) so the echo and the write pass can +//! never disagree about what counts as "an unbound matching tab". + +use jian_ops_schema::node::PenNode; +use op_editor_core::{EditorState, PenNodeExt}; + +use crate::wire_screen_navigation::{ + collect_nav_containers, first_text_content, labels_match, node_has_events, normalize_label, +}; + +/// A top-level `Frame` already carrying an authored `screen` route. +struct MarkedScreen<'a> { + node: &'a PenNode, + name: String, + path: String, +} + +/// Top-level `Frame` children that already carry an authored `screen` marker +/// — i.e. screens the model itself has already committed to routing, as +/// opposed to Track A's own width/height-shape heuristic (this echo takes no +/// position on frames the model hasn't marked yet). +fn marked_screens(nodes: &[PenNode]) -> Vec> { + nodes + .iter() + .filter_map(|node| { + let PenNode::Frame(frame) = node else { + return None; + }; + let path = frame.screen.clone()?; + let name = frame + .base + .name + .clone() + .unwrap_or_else(|| frame.base.id.clone()); + Some(MarkedScreen { node, name, path }) + }) + .collect() +} + +/// Scan the active page for nav-tab items that name-match an already +/// screen-marked frame but carry no `events` yet. Returns one human-readable +/// line per unbound-but-matched tab, naming the node id, the screen it sits +/// on, and the exact `events.onTap` patch to bind. No-ops (returns empty) +/// when fewer than two top-level frames are screen-marked — mirrors +/// `wire_screen_navigation`'s own multi-screen gate: a single marked screen +/// has no navigation target to check against yet. +pub fn scan_nav_issues(state: &EditorState) -> Vec { + let screens = marked_screens(state.active_children()); + if screens.len() < 2 { + return Vec::new(); + } + let targets: Vec<(String, String)> = screens + .iter() + .map(|s| (normalize_label(&s.name), s.path.clone())) + .collect(); + + let mut issues = Vec::new(); + for screen in &screens { + let mut nav_containers = Vec::new(); + collect_nav_containers(screen.node, &mut nav_containers); + for nav in nav_containers { + let Some(items) = nav.children() else { + continue; + }; + for item in items { + if node_has_events(item) { + continue; + } + let Some(label) = first_text_content(item) else { + continue; + }; + let tab_key = normalize_label(label); + let Some((_, target_path)) = targets + .iter() + .find(|(screen_key, _)| labels_match(&tab_key, screen_key)) + else { + continue; + }; + let item_id = item.id_str(); + let screen_path = &screen.path; + issues.push(format!( + "{item_id} (\"{label}\") on screen \"{screen_path}\" is not bound to \ + events.onTap yet - bind events:{{\"onTap\":[{{\"replace\":\"\\\"{target_path}\\\"\"}}]}} \ + so tapping it navigates to that screen" + )); + } + } + } + issues.sort(); + issues.truncate(8); + issues +} + +#[cfg(test)] +#[path = "nav_issues_tests.rs"] +mod tests; diff --git a/crates/op-orchestrator/src/nav_issues_tests.rs b/crates/op-orchestrator/src/nav_issues_tests.rs new file mode 100644 index 000000000..fd4b285fc --- /dev/null +++ b/crates/op-orchestrator/src/nav_issues_tests.rs @@ -0,0 +1,116 @@ +//! Tests for the Track B `navIssues` echo — see `nav_issues.rs` module doc. + +use super::*; +use jian_ops_schema::PenDocument; +use op_editor_core::EditorState; + +fn state_from_json(json: &str) -> EditorState { + let doc: PenDocument = serde_json::from_str(json).expect("valid PenDocument"); + EditorState::from_document(doc) +} + +/// Two screen-marked frames, each with a bottom-tab-bar. The "Home" tab is +/// already bound to itself (as `wire_screen_navigation` would leave it once +/// wired); the "Profile" tab has no `events` yet and its label matches the +/// OTHER screen's name, so it alone should be echoed with the node id and +/// the exact patch to bind. +const TWO_SCREENS_UNBOUND_PROFILE_TAB: &str = r##"{ "version": "1.0", "children": [ + { "type": "frame", "id": "home", "name": "Home", "screen": "/", + "width": 390, "height": 844, "layout": "vertical", + "children": [ + { "type": "frame", "id": "nav", "name": "Bottom Nav", "role": "bottom-tab-bar", + "layout": "horizontal", "width": "fill_container", + "children": [ + { "type": "frame", "id": "tab-home", "layout": "vertical", + "events": { "onTap": [ { "replace": "\"/\"" } ] }, + "children": [ { "type": "text", "id": "t1", "content": "Home" } ] }, + { "type": "frame", "id": "tab-profile", "layout": "vertical", + "children": [ { "type": "text", "id": "t2", "content": "Profile" } ] } + ] } + ] }, + { "type": "frame", "id": "profile", "name": "Profile", "screen": "/profile", + "width": 390, "height": 844 } +] }"##; + +#[test] +fn unbound_matching_tab_is_echoed_with_id_and_suggested_patch() { + let state = state_from_json(TWO_SCREENS_UNBOUND_PROFILE_TAB); + let issues = scan_nav_issues(&state); + assert_eq!(issues.len(), 1, "{issues:?}"); + assert!(issues[0].contains("tab-profile"), "{issues:?}"); + assert!( + issues[0].contains("\"/\""), + "names the screen it sits on: {issues:?}" + ); + assert!( + issues[0].contains(r#"{"replace":"\"/profile\""}"#), + "carries the exact bindable patch: {issues:?}" + ); +} + +#[test] +fn already_bound_tab_is_not_echoed() { + let mut state = state_from_json(TWO_SCREENS_UNBOUND_PROFILE_TAB); + // Bind the profile tab directly on the document before scanning. + let home = &mut state.active_children_mut()[0]; + let nav = &mut home.children_mut().unwrap()[0]; + let tab_profile = &mut nav.children_mut().unwrap()[1]; + let PenNode::Frame(tab_profile) = tab_profile else { + panic!("tab-profile frame"); + }; + tab_profile.events = Some( + serde_json::from_value(serde_json::json!({"onTap": [{"replace": "\"/profile\""}]})) + .unwrap(), + ); + + let issues = scan_nav_issues(&state); + assert!( + issues.is_empty(), + "already-bound tab must not be echoed: {issues:?}" + ); +} + +#[test] +fn single_marked_screen_has_no_navigation_to_check_yet() { + let state = state_from_json( + r##"{ "version": "1.0", "children": [ + { "type": "frame", "id": "home", "name": "Home", "screen": "/", + "width": 390, "height": 844, "layout": "vertical", + "children": [ + { "type": "frame", "id": "nav", "name": "Bottom Nav", "role": "bottom-tab-bar", + "layout": "horizontal", "width": "fill_container", + "children": [ + { "type": "frame", "id": "tab-profile", "layout": "vertical", + "children": [ { "type": "text", "id": "t2", "content": "Profile" } ] } + ] } + ] }, + { "type": "frame", "id": "profile", "name": "Profile", "width": 390, "height": 844 } + ] }"##, + ); + // Only "Home" is screen-marked — "Profile" is a plain frame, not yet + // committed to routing, so the gate must stay closed. + assert!(scan_nav_issues(&state).is_empty()); +} + +#[test] +fn unmatched_tab_label_is_not_echoed() { + let state = state_from_json( + r##"{ "version": "1.0", "children": [ + { "type": "frame", "id": "home", "name": "Home", "screen": "/", + "width": 390, "height": 844, "layout": "vertical", + "children": [ + { "type": "frame", "id": "nav", "name": "Bottom Nav", "role": "bottom-tab-bar", + "layout": "horizontal", "width": "fill_container", + "children": [ + { "type": "frame", "id": "tab-settings", "layout": "vertical", + "children": [ { "type": "text", "id": "t3", "content": "Settings" } ] } + ] } + ] }, + { "type": "frame", "id": "profile", "name": "Profile", "screen": "/profile", + "width": 390, "height": 844 } + ] }"##, + ); + // "Settings" matches neither "Home" nor "Profile" — ambiguous, so the + // echo stays silent rather than guessing a wrong destination. + assert!(scan_nav_issues(&state).is_empty()); +} diff --git a/crates/op-orchestrator/src/wire_screen_navigation.rs b/crates/op-orchestrator/src/wire_screen_navigation.rs new file mode 100644 index 000000000..80d7b39d1 --- /dev/null +++ b/crates/op-orchestrator/src/wire_screen_navigation.rs @@ -0,0 +1,525 @@ +//! Track A of the interactive-preview plan — deterministic screen/nav wiring. +//! +//! The preview engine (PreviewSession + jian-core `ScreenRouter` App Mode) +//! already understands multi-screen documents: it looks for top-level +//! `FrameNode.screen` markers and `events.onTap` navigation actions. What is +//! missing is generation-side wiring — AI-produced documents never carry +//! these fields, so `project_screens` finds nothing and preview degrades to +//! a single scrolling page. This pass fills that gap deterministically, with +//! zero model cooperation required (see +//! `openpencil-docs/openpencil/generation/preview-interactive-app-mode-0712.md`, +//! "Track A contract v2"). +//! +//! Hard contract (violating any of these breaks the preview engine, not just +//! this pass's own tests): +//! 1. A navigate action body is a Tier-1 EXPRESSION source, not a bare path — +//! `Expression::compile` lexes an unquoted `/x` as a division token and +//! fails to compile. The body must be the JSON string `"\"/path\""` (i.e. +//! the string VALUE is itself `"/path"` including the quote characters), +//! so `push`/`replace` bind to a string-literal expression. `pop` takes no +//! body (`null`). Verified against `jian-core/tests/action_navigation.rs`. +//! 2. `route` is never written — it is schema-only surface metadata that the +//! gesture dispatcher does not consume; only `events.onTap` drives runtime +//! navigation. +//! 3. `screen` only ever marks a top-level (page-root-level) frame — the +//! projection pass (`jian_ops_schema::screen_projection`) only scans +//! top-level children per page (or per-document when pageless). +//! 4. Idempotent + additive-only: a node that already carries `screen` or +//! `events` is never touched (an authored marker may be a future +//! breakpoint-variant of the same path — see the jian +//! `feat/responsive-m1a` compatibility notes in the plan doc). Running +//! the pass twice must be a no-op the second time. +//! 5. Zero new schema fields — only the existing `screen` / `events` fields +//! are ever written. +//! +//! ## Callers (`pub`, not `pub(crate)`) +//! +//! 1. `crate::cleanup::run_cleanup_passes` — the in-crate generation-pipeline +//! caller (orchestrator per-subtask cleanup + the agentic loop's whole-doc +//! finalize), which is why the pass writes through the crate's own +//! `DocSink` rather than a concrete document type. +//! 2. `op_host_native::preview::auto_wire` (Track C-1) — enter-preview +//! auto-wiring. When a document carries no authored `screen` marker at +//! all, the preview host runs this SAME pass over a JSON-cloned +//! `EditorState` before building the runtime, so a hand-drawn or +//! pre-Track-A multi-screen document still enters App Mode preview with +//! zero model cooperation. The saved document is never touched — see +//! `op-host-native/src/preview/mod.rs`'s "never mutates the saved doc" +//! invariant. + +use std::collections::{BTreeSet, HashMap}; + +use jian_ops_schema::node::{PenNode, TextContent}; +use op_editor_core::{EditorCommand, EditorState, NodeId, PenNodeExt}; + +use crate::types::DocSink; + +/// "Screen-shaped" top-level frame width bands: phone/narrow-tablet portrait +/// widths, or desktop-and-up. Chosen to gate OUT ordinary section widths +/// (e.g. a 600-900px card row) that are not standalone app screens. +const MOBILE_SCREEN_WIDTH: std::ops::RangeInclusive = 320.0..=480.0; +const DESKTOP_SCREEN_MIN_WIDTH: f64 = 1024.0; + +/// A back control must resolve within this many px of its screen's top edge +/// to count as "header region" — generous enough to cover a padded header +/// band (56-96px measured) plus a nested icon's own inset, tight enough that +/// a mid-page control is never mistaken for a nav-bar back arrow. +const HEADER_REGION_MAX_Y: f64 = 140.0; + +const ENTRY_NAME_HINTS: [&str; 4] = ["home", "main", "dashboard", "index"]; + +/// One screen-shaped top-level frame collected from the active page. +struct ScreenCandidate { + id: String, + /// Display name used for slugging + navbar label matching; falls back to + /// the node id when unnamed. + name: String, + /// Pre-existing authored `screen` marker, if any — never overwritten. + existing_path: Option, +} + +/// Entry point: mark screen-shaped top-level frames with a `screen` route +/// path and wire each screen's bottom-nav / sidebar-nav tabs + header back +/// buttons to `events.onTap` navigation actions. No-ops when the document +/// has fewer than two screen-shaped top-level frames (single-screen docs +/// keep today's scrolling-page preview — zero regression surface). +pub fn wire_screen_navigation(sink: &mut dyn DocSink) { + let screens = collect_screen_candidates(sink.state()); + if screens.len() < 2 { + return; + } + + let assignments = assign_screen_paths(&screens); + for (node_id, path) in &assignments { + sink.apply(EditorCommand::PatchNodeData { + node_id: NodeId::new(node_id.clone()), + patch_json: format!(r#"{{"screen":"{path}"}}"#), + page_id: None, + }); + } + + // Full id -> (path, name) table: authored markers keep their path, fresh + // assignments add theirs. Every candidate has exactly one entry. + let assigned: HashMap<&str, &str> = assignments + .iter() + .map(|(id, path)| (id.as_str(), path.as_str())) + .collect(); + let screen_paths: Vec<(String, String)> = screens + .iter() + .map(|s| { + let path = s + .existing_path + .clone() + .or_else(|| assigned.get(s.id.as_str()).map(|p| p.to_string())) + .unwrap_or_default(); + (s.name.clone(), path) + }) + .collect(); + + wire_nav_tabs(sink, &screens, &screen_paths); + wire_back_buttons(sink, &screens); +} + +/// Scan the active page's top-level `Frame` children for screen-shaped +/// candidates (numeric width AND height, width in a mobile or desktop band). +fn collect_screen_candidates(state: &EditorState) -> Vec { + state + .active_children() + .iter() + .filter_map(|node| { + let PenNode::Frame(frame) = node else { + return None; + }; + let width = node.width_px()?; + let height = node.height_px()?; + if height <= 0.0 { + return None; + } + if !(MOBILE_SCREEN_WIDTH.contains(&width) || width >= DESKTOP_SCREEN_MIN_WIDTH) { + return None; + } + Some(ScreenCandidate { + id: frame.base.id.clone(), + name: frame + .base + .name + .clone() + .unwrap_or_else(|| frame.base.id.clone()), + existing_path: frame.screen.clone(), + }) + }) + .collect() +} + +/// Assign a unique `/slug` path to every candidate that lacks an authored +/// `screen` marker. Exactly one candidate becomes the `"/"` entry — unless an +/// authored marker already claims `"/"`, in which case no new entry is +/// picked (an authored marker is never touched, even to satisfy the +/// single-entry rule; see contract point 4). Returns `(node_id, path)` +/// pairs to patch. +fn assign_screen_paths(candidates: &[ScreenCandidate]) -> Vec<(String, String)> { + let mut used: BTreeSet = candidates + .iter() + .filter_map(|c| c.existing_path.clone()) + .collect(); + let unmarked: Vec = candidates + .iter() + .enumerate() + .filter(|(_, c)| c.existing_path.is_none()) + .map(|(i, _)| i) + .collect(); + if unmarked.is_empty() { + return Vec::new(); + } + + let mut out = Vec::new(); + let entry_idx = if used.contains("/") { + None + } else { + Some( + unmarked + .iter() + .copied() + .find(|&i| is_entry_name(&candidates[i].name)) + .unwrap_or(unmarked[0]), + ) + }; + if let Some(idx) = entry_idx { + used.insert("/".to_string()); + out.push((candidates[idx].id.clone(), "/".to_string())); + } + + let mut fallback_index = 0usize; + for &i in &unmarked { + if Some(i) == entry_idx { + continue; + } + fallback_index += 1; + let slug = normalize_slug(&candidates[i].name); + let path = unique_path(&slug, fallback_index, &mut used); + out.push((candidates[i].id.clone(), path)); + } + out +} + +fn is_entry_name(name: &str) -> bool { + let slug = normalize_slug(name); + ENTRY_NAME_HINTS.iter().any(|hint| slug.contains(hint)) +} + +/// Lowercase ASCII-alnum slug with single hyphens between runs — non-ASCII +/// (CJK, emoji) and punctuation are stripped, not transliterated. An empty +/// result (all-non-ASCII name) falls back to `screen-N` in [`unique_path`]. +fn normalize_slug(name: &str) -> String { + let mut out = String::new(); + let mut pending_sep = false; + for ch in name.chars() { + if ch.is_ascii_alphanumeric() { + if pending_sep && !out.is_empty() { + out.push('-'); + } + out.push(ch.to_ascii_lowercase()); + pending_sep = false; + } else { + pending_sep = true; + } + } + out +} + +fn unique_path(slug: &str, fallback_index: usize, used: &mut BTreeSet) -> String { + let base = if slug.is_empty() { + format!("screen-{fallback_index}") + } else { + slug.to_string() + }; + let mut path = format!("/{base}"); + let mut suffix = 2; + while used.contains(&path) { + path = format!("/{base}-{suffix}"); + suffix += 1; + } + used.insert(path.clone()); + path +} + +// ── Navbar wiring ─────────────────────────────────────────────────────── + +/// Bind every screen's bottom-tab-bar / sidebar-nav tab items to `replace` +/// navigation toward the screen whose name matches the tab's label — +/// including a screen's own tab pointing back at itself (each screen wires +/// its navbar independently). A tab already carrying `events` is left alone. +fn wire_nav_tabs( + sink: &mut dyn DocSink, + screens: &[ScreenCandidate], + screen_paths: &[(String, String)], +) { + let normalized_screens: Vec<(String, String)> = screen_paths + .iter() + .filter(|(_, path)| !path.is_empty()) + .map(|(name, path)| (normalize_label(name), path.clone())) + .collect(); + + let mut patches: Vec<(String, String)> = Vec::new(); + for screen in screens { + let Some(root) = op_editor_core::walkers::find_node( + sink.state().active_children(), + &NodeId::new(screen.id.clone()), + ) else { + continue; + }; + let mut nav_containers = Vec::new(); + collect_nav_containers(root, &mut nav_containers); + for nav in nav_containers { + let Some(items) = nav.children() else { + continue; + }; + for item in items { + if node_has_events(item) { + continue; + } + let Some(label) = first_text_content(item) else { + continue; + }; + let tab_key = normalize_label(label); + let matched_path = normalized_screens + .iter() + .find(|(screen_key, _)| labels_match(&tab_key, screen_key)) + .map(|(_, path)| path.clone()); + let Some(path) = matched_path else { + continue; + }; + patches.push((item.id_str().to_string(), navigate_patch("replace", &path))); + } + } + } + for (node_id, patch_json) in patches { + sink.apply(EditorCommand::PatchNodeData { + node_id: NodeId::new(node_id), + patch_json, + page_id: None, + }); + } +} + +/// `pub` (not just module-private) so both the read-only `navIssues` echo +/// scan (`nav_issues.rs`, in-crate) and `op-smoke`'s `audit_rubric` +/// (cross-crate — op-smoke already depends on `op-orchestrator`) can reuse +/// the exact same nav-container detection this pass uses to WRITE — every +/// consumer that asks "is this a nav container" must agree with the pass +/// that actually binds the tabs inside one, or the rubric's +/// `navBoundTabs`/`navTotalTabs` columns would silently drift from what +/// Track A really wired. +pub fn collect_nav_containers<'a>(node: &'a PenNode, out: &mut Vec<&'a PenNode>) { + if is_nav_container(node) { + out.push(node); + } + for child in node.children().into_iter().flatten() { + collect_nav_containers(child, out); + } +} + +fn is_nav_container(node: &PenNode) -> bool { + let role = node + .base() + .role + .as_deref() + .unwrap_or("") + .to_ascii_lowercase(); + if matches!( + role.as_str(), + "nav" | "tab-bar" | "bottom-tab-bar" | "tab-row" | "sidebar" | "side-nav" | "nav-rail" + ) { + return true; + } + let hay = identity_haystack(node); + [ + "bottom nav", + "bottom-nav", + "bottom navigation", + "bottom-navigation", + "tab bar", + "tab-bar", + "sidebar", + "side nav", + "side-nav", + "nav rail", + "nav-rail", + ] + .iter() + .any(|needle| hay.contains(needle)) +} + +fn identity_haystack(node: &PenNode) -> String { + format!( + "{} {}", + node.id_str().to_ascii_lowercase(), + node.base() + .name + .as_deref() + .unwrap_or("") + .to_ascii_lowercase() + ) +} + +/// First non-empty plain-text content found via depth-first search — the +/// tab-item label (icon + label rows are the universal shape; styled/rich +/// text is skipped rather than guessed at). `pub(crate)` for the `navIssues` +/// echo scan — see [`collect_nav_containers`]. +pub(crate) fn first_text_content(node: &PenNode) -> Option<&str> { + if let PenNode::Text(text) = node { + return match &text.content { + TextContent::Plain(s) if !s.trim().is_empty() => Some(s.as_str()), + _ => None, + }; + } + node.children()?.iter().find_map(first_text_content) +} + +/// `pub(crate)` for the `navIssues` echo scan — see [`collect_nav_containers`]. +pub(crate) fn normalize_label(s: &str) -> String { + s.chars() + .filter(|c| c.is_ascii_alphanumeric()) + .flat_map(|c| c.to_lowercase()) + .collect() +} + +/// Match iff normalized forms are equal or one is a prefix of the other +/// (covers "Profile" vs "Profile Screen" → "profile" / "profilescreen"). +/// Ambiguous (neither) never binds — a wrong navigate is worse than a dead +/// tap. `pub(crate)` for the `navIssues` echo scan — see +/// [`collect_nav_containers`]. +pub(crate) fn labels_match(a: &str, b: &str) -> bool { + !a.is_empty() && !b.is_empty() && (a == b || a.starts_with(b) || b.starts_with(a)) +} + +// ── Back-button wiring ────────────────────────────────────────────────── + +/// Bind a header-region back control (name/icon containing back / +/// arrow-left / chevron-left) to `pop`. There is no system-level back +/// affordance in v1 (design §7) — only an authored UI node can carry it. +fn wire_back_buttons(sink: &mut dyn DocSink, screens: &[ScreenCandidate]) { + let y_offsets = resolved_y_offsets(sink.state()); + let mut patches: Vec = Vec::new(); + for screen in screens { + let Some(root) = op_editor_core::walkers::find_node( + sink.state().active_children(), + &NodeId::new(screen.id.clone()), + ) else { + continue; + }; + let screen_top = y_offsets.get(&screen.id).copied().unwrap_or(0.0); + collect_back_controls(root, screen_top, &y_offsets, &mut patches); + } + for node_id in patches { + sink.apply(EditorCommand::PatchNodeData { + node_id: NodeId::new(node_id), + patch_json: r#"{"events":{"onTap":[{"pop":null}]}}"#.to_string(), + page_id: None, + }); + } +} + +fn collect_back_controls( + node: &PenNode, + screen_top: f64, + y_offsets: &HashMap, + out: &mut Vec, +) { + if node_has_events(node) { + // An authored interactive control owns its whole subtree — wiring a + // descendant underneath (e.g. the arrow icon inside an already-bound + // back button) would double-dispatch the same tap. + return; + } + if is_back_control(node) { + let within_header = y_offsets + .get(node.id_str()) + .is_some_and(|y| (y - screen_top) <= HEADER_REGION_MAX_Y); + if within_header { + out.push(node.id_str().to_string()); + return; // don't also bind a nested icon inside the matched control + } + } + for child in node.children().into_iter().flatten() { + collect_back_controls(child, screen_top, y_offsets, out); + } +} + +fn is_back_control(node: &PenNode) -> bool { + if matches!( + node.base().role.as_deref(), + Some("back" | "back-button" | "nav-back") + ) { + return true; + } + let mut hay = compact_lower(node.base().name.as_deref().unwrap_or("")); + hay.push(' '); + hay.push_str(&compact_lower(node.id_str())); + if let PenNode::IconFont(icon) = node { + hay.push(' '); + hay.push_str(&compact_lower(&icon.icon_font_name)); + } + hay.contains("back") || hay.contains("arrowleft") || hay.contains("chevronleft") +} + +fn compact_lower(s: &str) -> String { + s.chars() + .filter(|c| c.is_ascii_alphanumeric()) + .flat_map(|c| c.to_lowercase()) + .collect() +} + +// ── Shared helpers ────────────────────────────────────────────────────── + +/// Does this node already carry a (non-empty) `events` block? Checked via +/// JSON rather than a per-variant match since every actionable node variant +/// (Frame/Group/Rectangle/Text/IconFont/…) carries the same optional field — +/// idempotency only needs "is it present", not which handler. `pub` for the +/// in-crate `navIssues` echo scan AND `op-smoke`'s `audit_rubric` +/// (`navBoundTabs`) — see [`collect_nav_containers`]. +pub fn node_has_events(node: &PenNode) -> bool { + serde_json::to_value(node) + .ok() + .and_then(|v| v.get("events").cloned()) + .is_some() +} + +/// Build the `events.onTap` navigate patch JSON. `path` must already be a +/// `/`-rooted route path; the JSON string VALUE is the literal +/// `""` (quotes included) so it compiles as a Tier-1 string-literal +/// expression — see the module doc, contract point 1. +fn navigate_patch(verb: &str, path: &str) -> String { + let body = serde_json::to_string(path).unwrap_or_default(); // -> "\"/path\"" + let escaped_body = serde_json::to_string(&body).unwrap_or_default(); // -> "\"\\\"/path\\\"\"" + format!(r#"{{"events":{{"onTap":[{{"{verb}":{escaped_body}}}]}}}}"#) +} + +/// Node id -> resolved absolute-doc-space Y offset, via the same jian layout +/// pass `snapshot_layout` / `geometry_validation` use. Only Y is needed here +/// (header-region gating); a fuller Rect lives in `geometry_validation` +/// scoped to its own module, mirroring the existing per-module +/// `resolved_widths` precedent in `sidebar_archetype`. +fn resolved_y_offsets(state: &EditorState) -> HashMap { + let scene = op_pen_loader::editor_state_to_layout_scene(state); + let mut out = HashMap::new(); + for page in &scene.pages { + collect_y_offsets(&page.children, &mut out); + } + out +} + +fn collect_y_offsets( + nodes: &[jian_scene::layout_scene::SceneNode], + out: &mut HashMap, +) { + for node in nodes { + let bounds = node.aggregate_bounds(); + out.insert(node.id.clone(), f64::from(bounds.origin.y)); + collect_y_offsets(&node.children, out); + } +} + +#[cfg(test)] +#[path = "wire_screen_navigation_tests.rs"] +mod tests; diff --git a/crates/op-orchestrator/src/wire_screen_navigation_tests.rs b/crates/op-orchestrator/src/wire_screen_navigation_tests.rs new file mode 100644 index 000000000..f079892a9 --- /dev/null +++ b/crates/op-orchestrator/src/wire_screen_navigation_tests.rs @@ -0,0 +1,384 @@ +//! Tests for Track A's deterministic screen/nav wiring pass. + +use super::*; +use jian_ops_schema::PenDocument; +use op_editor_core::EditorState; + +fn state_from_json(json: &str) -> EditorState { + let doc: PenDocument = serde_json::from_str(json).expect("valid PenDocument"); + EditorState::from_document(doc) +} + +fn find_by_id<'a>(nodes: &'a [PenNode], id: &str) -> Option<&'a PenNode> { + for node in nodes { + if node.id_str() == id { + return Some(node); + } + if let Some(children) = node.children() { + if let Some(found) = find_by_id(children, id) { + return Some(found); + } + } + } + None +} + +fn frame_screen(node: &PenNode) -> Option<&str> { + match node { + PenNode::Frame(f) => f.screen.as_deref(), + _ => None, + } +} + +fn node_events_json(node: &PenNode) -> Option { + serde_json::to_value(node).ok()?.get("events").cloned() +} + +fn run_pass(state: &mut EditorState) { + let mut sink = crate::loop_finalize::StateDocSink { state }; + wire_screen_navigation(&mut sink); +} + +// ── Pure-helper unit tests ───────────────────────────────────────────── + +#[test] +fn normalize_slug_strips_non_ascii_and_hyphenates() { + assert_eq!(normalize_slug("Profile Screen"), "profile-screen"); + assert_eq!(normalize_slug(" Settings!! "), "settings"); + assert_eq!(normalize_slug("首页"), ""); // all-CJK -> empty, caller falls back +} + +#[test] +fn labels_match_exact_and_prefix() { + assert!(labels_match("profile", "profile")); + assert!(labels_match("profile", "profilescreen")); // "Profile" vs "Profile Screen" + assert!(labels_match("profilescreen", "profile")); + assert!(!labels_match("profile", "settings")); + assert!(!labels_match("", "profile")); +} + +/// Contract point 1: the navigate body must be a Tier-1 string-LITERAL +/// expression source — a bare `/path` lexes as division and fails to +/// compile (`Expression::compile`). The correct wire form is exactly the +/// shape asserted by jian-core's own fixtures +/// (`jian-core/tests/action_navigation.rs::push_literal_path`, +/// `jian-ops-schema/src/events.rs::push_action_with_string_body`): the +/// JSON value under `"replace"`/`"push"` is the STRING `"\"/path\""` (i.e. +/// its decoded content is `"/path"`, quote characters included). +#[test] +fn navigate_patch_produces_expression_literal_body() { + let patch = navigate_patch("replace", "/profile"); + assert_eq!( + patch, + r#"{"events":{"onTap":[{"replace":"\"/profile\""}]}}"# + ); + // Round-trip through a real JSON parser to double-check the DECODED + // action body equals the 10-char string `"/profile"` (quotes included) — + // exactly what `jian_core::action::actions::navigation::expr_from_value` + // feeds to `Expression::compile`. + let v: serde_json::Value = serde_json::from_str(&patch).unwrap(); + let body = v["events"]["onTap"][0]["replace"].as_str().unwrap(); + assert_eq!(body, "\"/profile\""); +} + +// ── Gate ──────────────────────────────────────────────────────────────── + +#[test] +fn single_screen_is_untouched_zero_new_keys() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + let before = serde_json::to_string(state.active_children()).unwrap(); + run_pass(&mut state); + let after = serde_json::to_string(state.active_children()).unwrap(); + assert_eq!( + after, before, + "gate: <2 screen-shaped frames must be a no-op" + ); + assert!(!after.contains("screen"), "no new `screen` key grown"); +} + +// ── Screen marking ────────────────────────────────────────────────────── + +#[test] +fn two_screens_get_entry_and_slug_path() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]}, + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let home = find_by_id(state.active_children(), "home").unwrap(); + let profile = find_by_id(state.active_children(), "profile").unwrap(); + assert_eq!(frame_screen(home), Some("/")); + assert_eq!(frame_screen(profile), Some("/profile")); +} + +#[test] +fn entry_prefers_home_like_name_over_doc_order() { + // "Profile" is first in document order, but "Dashboard" carries an + // entry-name hint — Dashboard must win "/" regardless of position. + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[]}, + {"type":"frame","id":"dash","name":"Dashboard","width":1200,"height":900, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let profile = find_by_id(state.active_children(), "profile").unwrap(); + let dash = find_by_id(state.active_children(), "dash").unwrap(); + assert_eq!(frame_screen(dash), Some("/")); + assert_eq!(frame_screen(profile), Some("/profile")); +} + +#[test] +fn duplicate_slugs_get_numeric_suffix() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]}, + {"type":"frame","id":"settings-a","name":"Settings","width":390,"height":844, + "layout":"vertical","children":[]}, + {"type":"frame","id":"settings-b","name":"Settings","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let a = find_by_id(state.active_children(), "settings-a").unwrap(); + let b = find_by_id(state.active_children(), "settings-b").unwrap(); + // Doc-order first gets the bare slug, the second gets the `-2` suffix. + assert_eq!(frame_screen(a), Some("/settings")); + assert_eq!(frame_screen(b), Some("/settings-2")); +} + +#[test] +fn authored_screen_marker_is_never_overwritten_and_not_reused_as_entry() { + // "Checkout" already carries an authored (non-"/") marker. Since the + // pass never touches authored markers even to satisfy the single-entry + // rule, "/" is picked only among the UNMARKED candidates — here that's + // "Extras", the only other one, despite its name matching no entry hint. + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"checkout","name":"Checkout","width":390,"height":844, + "layout":"vertical","children":[],"screen":"/checkout"}, + {"type":"frame","id":"extras","name":"Extras","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let checkout = find_by_id(state.active_children(), "checkout").unwrap(); + let extras = find_by_id(state.active_children(), "extras").unwrap(); + assert_eq!( + frame_screen(checkout), + Some("/checkout"), + "authored marker untouched" + ); + assert_eq!( + frame_screen(extras), + Some("/"), + "sole unmarked candidate becomes entry" + ); +} + +#[test] +fn second_run_is_idempotent() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"nav-home","name":"Bottom Nav","role":"bottom-tab-bar", + "width":"fill_container","height":56,"layout":"horizontal","children":[ + {"type":"frame","id":"tab-home","name":"HomeTab","width":80,"height":40, + "children":[{"type":"text","id":"tab-home-lbl","content":"Home"}]}, + {"type":"frame","id":"tab-profile","name":"ProfileTab","width":80,"height":40, + "children":[{"type":"text","id":"tab-profile-lbl","content":"Profile"}]} + ]} + ]}, + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let once = serde_json::to_string(state.active_children()).unwrap(); + run_pass(&mut state); + let twice = serde_json::to_string(state.active_children()).unwrap(); + assert_eq!( + once, twice, + "running the pass twice must be a no-op the second time" + ); +} + +// ── Navbar wiring ─────────────────────────────────────────────────────── + +fn two_screen_bottom_nav_doc() -> &'static str { + r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"nav-home","name":"Bottom Nav","role":"bottom-tab-bar", + "width":"fill_container","height":56,"layout":"horizontal","children":[ + {"type":"frame","id":"tab-home-in-home","name":"HomeTab","width":80,"height":40, + "children":[{"type":"text","id":"t1","content":"Home"}]}, + {"type":"frame","id":"tab-profile-in-home","name":"ProfileTab","width":80,"height":40, + "children":[{"type":"text","id":"t2","content":"Profile Screen"}]}, + {"type":"frame","id":"tab-settings-in-home","name":"SettingsTab","width":80,"height":40, + "children":[{"type":"text","id":"t3","content":"Settings"}]} + ]} + ]}, + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"nav-profile","name":"Bottom Nav","role":"bottom-tab-bar", + "width":"fill_container","height":56,"layout":"horizontal","children":[ + {"type":"frame","id":"tab-home-in-profile","name":"HomeTab","width":80,"height":40, + "children":[{"type":"text","id":"t4","content":"Home"}]}, + {"type":"frame","id":"tab-profile-in-profile","name":"ProfileTab","width":80,"height":40, + "children":[{"type":"text","id":"t5","content":"Profile Screen"}]} + ]} + ]} + ]}"# +} + +#[test] +fn bottom_nav_tabs_wire_to_matching_screens_including_self() { + let mut state = state_from_json(two_screen_bottom_nav_doc()); + run_pass(&mut state); + + let home_path = frame_screen(find_by_id(state.active_children(), "home").unwrap()) + .unwrap() + .to_string(); + let profile_path = frame_screen(find_by_id(state.active_children(), "profile").unwrap()) + .unwrap() + .to_string(); + + // Home screen's own navbar: Home tab points at itself, Profile tab at Profile. + let home_tab_in_home = find_by_id(state.active_children(), "tab-home-in-home").unwrap(); + let profile_tab_in_home = find_by_id(state.active_children(), "tab-profile-in-home").unwrap(); + assert_eq!( + node_events_json(home_tab_in_home).unwrap()["onTap"][0]["replace"], + serde_json::json!(format!("\"{home_path}\"")) + ); + assert_eq!( + node_events_json(profile_tab_in_home).unwrap()["onTap"][0]["replace"], + serde_json::json!(format!("\"{profile_path}\"")) + ); + + // Profile screen's own navbar wires the same way independently. + let home_tab_in_profile = find_by_id(state.active_children(), "tab-home-in-profile").unwrap(); + let profile_tab_in_profile = + find_by_id(state.active_children(), "tab-profile-in-profile").unwrap(); + assert_eq!( + node_events_json(home_tab_in_profile).unwrap()["onTap"][0]["replace"], + serde_json::json!(format!("\"{home_path}\"")) + ); + assert_eq!( + node_events_json(profile_tab_in_profile).unwrap()["onTap"][0]["replace"], + serde_json::json!(format!("\"{profile_path}\"")) + ); +} + +#[test] +fn mismatched_tab_label_is_not_bound() { + let mut state = state_from_json(two_screen_bottom_nav_doc()); + run_pass(&mut state); + // "Settings" has no matching screen (only Home/Profile exist) — its tab + // must stay unbound rather than guess. + let settings_tab = find_by_id(state.active_children(), "tab-settings-in-home").unwrap(); + assert!(node_events_json(settings_tab).is_none()); +} + +#[test] +fn existing_tab_events_are_left_alone() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"nav-home","name":"Bottom Nav","role":"bottom-tab-bar", + "width":"fill_container","height":56,"layout":"horizontal","children":[ + {"type":"frame","id":"tab-profile","name":"ProfileTab","width":80,"height":40, + "events":{"onTap":[{"custom_action":null}]}, + "children":[{"type":"text","id":"t1","content":"Profile"}]} + ]} + ]}, + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + let before = node_events_json(find_by_id(state.active_children(), "tab-profile").unwrap()); + run_pass(&mut state); + let after = node_events_json(find_by_id(state.active_children(), "tab-profile").unwrap()); + assert_eq!( + after, before, + "a node with existing events is never touched" + ); +} + +// ── Back-button wiring ────────────────────────────────────────────────── + +#[test] +fn header_back_icon_gets_pop() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"icon_font","id":"back-icon","name":"Back","iconFontName":"arrow-left", + "width":24,"height":24} + ]}, + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let back = find_by_id(state.active_children(), "back-icon").unwrap(); + assert_eq!( + node_events_json(back).unwrap(), + serde_json::json!({"onTap": [{"pop": null}]}) + ); +} + +/// An authored interactive control owns its whole subtree: the arrow icon +/// INSIDE an already-bound back button must not receive a second `pop` +/// (both handlers firing on one tap would double-pop the route stack). +#[test] +fn icon_inside_authored_back_button_is_not_double_bound() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"back-btn","name":"Back Button","width":44,"height":44, + "events":{"onTap":[{"pop":null}]}, + "children":[ + {"type":"icon_font","id":"inner-icon","name":"arrow-left","iconFontName":"arrow-left", + "width":24,"height":24} + ]} + ]}, + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let icon = find_by_id(state.active_children(), "inner-icon").unwrap(); + assert!( + node_events_json(icon).is_none(), + "a descendant of an authored interactive control must never be wired" + ); +} + +#[test] +fn back_icon_outside_header_region_is_not_bound() { + let json = r#"{"version":"1.0","children":[ + {"type":"frame","id":"profile","name":"Profile","width":390,"height":844, + "layout":"vertical","children":[ + {"type":"frame","id":"filler","name":"Filler","width":"fill_container","height":700, + "children":[]}, + {"type":"icon_font","id":"chevron-icon","name":"chevron-left","iconFontName":"chevron-left", + "width":24,"height":24} + ]}, + {"type":"frame","id":"home","name":"Home","width":390,"height":844, + "layout":"vertical","children":[]} + ]}"#; + let mut state = state_from_json(json); + run_pass(&mut state); + let chevron = find_by_id(state.active_children(), "chevron-icon").unwrap(); + assert!( + node_events_json(chevron).is_none(), + "a back-shaped icon well below the header band must not be bound" + ); +}