Merge pull request #1392 from buyun14/fix/batch-line-tail

fix(automation): consume and report the tail of a batch command line

benches/README.md

@@ -19,7 +19,7 @@ cargo bench --bench performance_benchmarks -- --quick
 
 ---
 
-## Benchmark Suite Catalog (25 Metrics)
+## Benchmark Suite Catalog (26 Metrics)
 
 ### 1. Scene & Entity Management
 - `scene_entity_ingestion_10k`: Ingestion throughput for 10,000 mixed 2D entities (Lines, Circles, Arcs, Polylines).
@@ -51,6 +51,7 @@ cargo bench --bench performance_benchmarks -- --quick
 - `ui_grid_cache_hit_evaluation`: Grid overlay cache key construction and reuse decision.
 - `ui_icon_handle_cached`: Themed SVG icon cache hit (FxHashMap lookup + Handle clone).
 - `ui_icon_handle_uncached`: Uncached SVG icon memory parse and Handle allocation.
+- `ui_plotstyle_layer_usage`: Plot Style 256-bucket ACI layer-usage table rebuild (200 layers).
 
 ### 6. State Management & Draworder
 - `selection_state_deep_clone`: Deep cloning of `SelectionState` (with 64 polygon points).

benches/performance_benchmarks.rs

@@ -13,6 +13,7 @@
 //! 10. UI Ribbon View Widget Tree Construction
 //! 11. UI Viewport Grid Geometry Projection & Overlay Cache Key Evaluation
 //! 12. UI Themed SVG Icon Lookup Caching vs Uncached Parse
+//! 12b. Plot Style Layer-Usage Table Rebuild (256-bucket ACI table)
 //! 13. Wide & Tapered Arc + Donut Tessellation (7.5k offset polyline curves)
 //! 14. Model Space Extents & Bounding Box Calculation (ZOOM EXTENTS)
 //! 15. Batch Entity Transformation & Incremental Dirty-Tracking
@@ -64,6 +65,7 @@ use OpenCADStudio::ui::overlay::{
 };
 use OpenCADStudio::ui::properties::LinetypeItem;
 use OpenCADStudio::ui::ribbon::{LayerInfo, Ribbon};
+use OpenCADStudio::ui::style::plotstyle::build_layer_usage;
 
 // ── Metric Structures ───────────────────────────────────────────────────────
 
@@ -1359,6 +1361,53 @@ fn bench_ui_icon_caching(runner: &mut BenchmarkRunner) {
     }
 }
 
+// ── 12b. Plot Style Layer-Usage Table Rebuild ───────────────────────────────
+// Covers Mission #19 `build_layer_usage`: layer names bucketed by ACI into a
+// 256-bucket table, rebuilt per Plot Style modal view.
+
+fn bench_ui_plotstyle_layer_usage(runner: &mut BenchmarkRunner) {
+    if !runner.should_run("ui_plotstyle_layer_usage") {
+        return;
+    }
+
+    let mut doc = CadDocument::new();
+    for i in 0..200 {
+        let mut layer = acadrust::tables::Layer::new(&format!("LAYER_{i:03}"));
+        layer.handle = doc.allocate_handle();
+        layer.color = acadrust::types::Color::Index((i % 8 + 1) as u8);
+        let _ = doc.layers.add(layer);
+    }
+
+    // Warm-up for allocator settling
+    for _ in 0..10 {
+        let _ = black_box(build_layer_usage(&doc));
+    }
+
+    let n = if runner.quick_mode { 20 } else { 100 };
+    let runs = 5;
+    let mut samples = Vec::with_capacity(runs);
+
+    for _ in 0..runs {
+        let t0 = Instant::now();
+        for _ in 0..n {
+            let usage = build_layer_usage(black_box(&doc));
+            black_box(usage);
+        }
+        let per_us = (t0.elapsed().as_micros() as f64) / (n as f64);
+        samples.push(per_us);
+    }
+
+    let median_us = samples[samples.len() / 2];
+    runner.record(
+        "ui_plotstyle_layer_usage",
+        "Plot Style 256-bucket ACI layer-usage table rebuild (200 layers, 8 buckets x25)",
+        "µs",
+        samples,
+        Some((1_000_000.0 / median_us, "rebuilds/s")),
+        Some(50.0), // Target threshold < 50 µs
+    );
+}
+
 // ── 13. Wide & Tapered Arc + Donut Tessellation ─────────────────────────────
 
 fn bench_wide_and_tapered_arc_tessellation(runner: &mut BenchmarkRunner) {
@@ -1761,6 +1810,7 @@ fn main() {
     bench_ui_ribbon_view_construction(&mut runner);
     bench_ui_grid_geometry(&mut runner);
     bench_ui_icon_caching(&mut runner);
+    bench_ui_plotstyle_layer_usage(&mut runner);
     bench_wide_and_tapered_arc_tessellation(&mut runner);
     bench_zoom_extents_calculation(&mut runner);
     bench_batch_entity_mutation(&mut runner);

docs/automation/README.md

@@ -138,7 +138,27 @@ Execute responses use `response_detail: "compact"` by default and return only th
 
 For unfamiliar or conditional commands, use `start`, then inspect `state.command` in every response. Its `accepts` array gives the valid MCP input kinds, `options` gives the current tokens, and `input_example` gives the next request shape. Add the current state fields and a new `request_id` to each step.
 
-Every `ocs_execute` request requires a caller-generated `request_id`. Reuse that ID only to retry the identical request after a timeout, together with the same session ID, document ID, expected revision, and selection. Commands report `waiting_input` while more input is required and asynchronous work remains `running` until its real callback finishes. Geometry stays in OpenCADStudio and its geometry kernel.
+Every `ocs_execute` request requires a caller-generated `request_id`. Reuse that ID only to retry the identical request after a timeout, together with the same session ID, document ID, expected revision, and selection. Commands report `waiting_input` while more input is required and asynchronous work remains `running` until its real callback finishes.
+
+A `run` reply also carries two fields that describe what happened to the line:
+
+* `blocked_by` — `null` when the command finished, otherwise what *this line*
+  left open and waiting: `"command"` (an interactive command), `"text_editor"`
+  (the in-place editor opened by the `TEXT` content step), `"mtext_editor"`, or
+  `"modal:<Kind>"` for a dialog. A surface that was already open when the line
+  started (an earlier line's editor, another tab, a startup dialog) is not
+  attributed to this line, so a caller polling for `completed` is never stuck
+  waiting on someone else's editor. `blocked_by` is set whenever `status` is
+  `waiting_input`.
+* `unconsumed` — the tokens of the line that no prompt ever asked for, in order.
+  A fully consumed line reports `[]`; `CIRCLE 0,0 5 9` reports `["9"]` because
+  the radius step ends the command. Over-typed input is reported instead of
+  being dropped on the floor.
+
+When a line ends at the `TEXT` content step (`TEXT 0,0 5 0 hello`) the text is
+typed into the in-place editor and committed, so the line behaves exactly as the
+same input typed into the GUI command line would: one `Text` entity, nothing
+unconsumed, and the command closed rather than left armed for a second line. Geometry stays in OpenCADStudio and its geometry kernel.
 
 `ocs_capture` defaults to the drawing viewport and a longest edge of 1600 pixels. Set `scope` to `window` for the full interface or change `max_dimension` between 256 and 4096.
 

src/app/automation.rs

@@ -350,6 +350,14 @@ impl OpenCADStudio {
                 let i = self.active_tab;
                 let before = self.tabs[i].scene.document.entities().count();
                 let error_revision = self.command_line.error_revision;
+                // Surfaces that are already open belong to an earlier line (or
+                // another tab, or startup). Only what *this* line left open is
+                // reported as a blocker, otherwise a finished command would keep
+                // answering `waiting_input` because of someone else's editor and
+                // a caller polling for `completed` would never get there.
+                let editor_before = self.text_inline.is_some();
+                let mtext_before = self.mtext_editor.is_some();
+                let modal_before = self.active_modal.is_some();
                 if let Err(error) = self.run_headless(&cmd) {
                     return err(error);
                 }
@@ -357,12 +365,32 @@ impl OpenCADStudio {
                     return err(self.command_line.last_error.clone().unwrap_or_default());
                 }
                 let after = self.tabs[i].scene.document.entities().count();
+                // A command can finish with an interactive surface still open:
+                // the in-place text editor (the `TEXT` content step), the MTEXT
+                // editor, or a modal. Reporting `completed` there hides the fact
+                // that nothing was committed, so reuse the `waiting_input`
+                // vocabulary and name the blocker.
+                let blocked_by: Option<String> = if self.tabs[i].active_cmd.is_some() {
+                    Some("command".to_string())
+                } else if let (false, Some(modal)) = (modal_before, self.active_modal.as_ref()) {
+                    Some(format!("modal:{modal:?}"))
+                } else if !editor_before && self.text_inline.is_some() {
+                    Some("text_editor".to_string())
+                } else if !mtext_before && self.mtext_editor.is_some() {
+                    Some("mtext_editor".to_string())
+                } else {
+                    None
+                };
                 json!({
                     "ok": true,
                     "cmd": cmd,
-                    "status": if self.tabs[i].active_cmd.is_some() { "waiting_input" } else { "completed" },
+                    "status": if blocked_by.is_some() { "waiting_input" } else { "completed" },
+                    "blocked_by": blocked_by,
                     "entities": after,
                     "added": after as i64 - before as i64,
+                    // Tokens no prompt ever asked for: leftover input is
+                    // reported rather than dropped on the floor.
+                    "unconsumed": self.command_line.unconsumed.clone(),
                 })
             }
             "entities" => self.entity_summary(),
@@ -772,6 +800,128 @@ mod tests {
         assert!(app.tabs[i].scene.last_tess_wires.get() > before);
     }
 
+    #[test]
+    fn batch_text_line_creates_text_and_consumes_every_token() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        // Regression: the content token used to be dropped once the in-place
+        // editor took over, so this line created nothing while reporting ok.
+        let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0 hello"}"#);
+        assert_eq!(r["ok"], true);
+        assert_eq!(r["status"], "completed", "{r}");
+        assert_eq!(r["added"], 1, "{r}");
+        assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}");
+
+        let counts = app.automation_op(r#"{"op":"entities"}"#);
+        assert_eq!(counts["by_type"]["Text"], 1, "{counts}");
+        assert_eq!(counts["total"], 1, "no phantom entity: {counts}");
+        assert_eq!(assert_one_text(&app), "hello");
+    }
+
+    #[test]
+    fn multi_word_text_keeps_the_spaces() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0 hello world"}"#);
+        assert_eq!(r["status"], "completed", "{r}");
+        assert_eq!(r["added"], 1, "{r}");
+        assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}");
+        let counts = app.automation_op(r#"{"op":"entities"}"#);
+        assert_eq!(counts["by_type"]["Text"], 1, "{counts}");
+
+        // The spaces between the words have to survive the token split, and the
+        // whole tail (not just its first token) has to land in the entity — an
+        // off-by-one in the consumed index would still pass the checks above.
+        assert_eq!(assert_one_text(&app), "hello world");
+    }
+
+    #[test]
+    fn a_stray_editor_is_not_hijacked_by_the_next_line() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        // Line 1 stops at the content step and leaves the editor open.
+        let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 30,0 5 0"}"#);
+        assert_eq!(r["blocked_by"], "text_editor", "{r}");
+
+        // Line 2 must not type its leftover token into that unrelated editor,
+        // and — since line 2 did not open anything itself — it must not inherit
+        // line 1's blocker either: a caller that polls for `completed` would
+        // otherwise never get there.
+        let r = app.automation_op(r#"{"op":"run","cmd":"CIRCLE 0,0 5 9"}"#);
+        assert_eq!(r["added"], 1, "{r}");
+        assert_eq!(r["unconsumed"][0], "9", "{r}");
+        assert_eq!(r["blocked_by"], serde_json::Value::Null, "{r}");
+        assert_eq!(r["status"], "completed", "{r}");
+        let counts = app.automation_op(r#"{"op":"entities"}"#);
+        assert_eq!(counts["by_type"]["Circle"], 1, "{counts}");
+        assert!(
+            counts["by_type"].get("Text").is_none(),
+            "phantom text from stray editor: {counts}"
+        );
+    }
+
+    #[test]
+    fn run_reports_leftover_tokens_instead_of_dropping_them() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        // The radius step ends the command, so the extra token is never asked for.
+        let r = app.automation_op(r#"{"op":"run","cmd":"CIRCLE 0,0 5 9"}"#);
+        assert_eq!(r["added"], 1, "{r}");
+        assert_eq!(r["unconsumed"][0], "9", "{r}");
+
+        // A fully consumed line reports nothing left over.
+        let r = app.automation_op(r#"{"op":"run","cmd":"LINE 0,0 10,10"}"#);
+        assert_eq!(r["unconsumed"].as_array().map(|v| v.len()), Some(0), "{r}");
+    }
+
+    #[test]
+    fn text_without_content_reports_the_open_editor() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        let r = app.automation_op(r#"{"op":"run","cmd":"TEXT 0,0 5 0"}"#);
+        assert_eq!(r["status"], "waiting_input", "{r}");
+        assert_eq!(r["blocked_by"], "text_editor", "{r}");
+        let counts = app.automation_op(r#"{"op":"entities"}"#);
+        assert_eq!(counts["total"], 0, "{counts}");
+
+        // The same editor can be finished through the control-surface messages.
+        let _ = app.update(crate::app::Message::TextInlineInput("hello".into()));
+        let _ = app.update(crate::app::Message::TextInlineOk);
+        let counts = app.automation_op(r#"{"op":"entities"}"#);
+        assert_eq!(counts["by_type"]["Text"], 1, "{counts}");
+    }
+
+    #[test]
+    fn a_command_still_waiting_is_named_as_the_blocker() {
+        let mut app = OpenCADStudio::new_for_test();
+        let _ = app.automation_op(r#"{"op":"new"}"#);
+
+        let r = app.automation_op(r#"{"op":"run","cmd":"LINE"}"#);
+        assert_eq!(r["status"], "waiting_input", "{r}");
+        assert_eq!(r["blocked_by"], "command", "{r}");
+    }
+
+    /// The text of the single `Text` entity in the drawing (panics otherwise).
+    fn assert_one_text(app: &OpenCADStudio) -> String {
+        let i = app.active_tab;
+        let texts: Vec<String> = app.tabs[i]
+            .scene
+            .document
+            .entities()
+            .filter_map(|e| match e {
+                acadrust::EntityType::Text(t) => Some(t.value.clone()),
+                _ => None,
+            })
+            .collect();
+        assert_eq!(texts.len(), 1, "expected exactly one Text entity, got {texts:?}");
+        texts.into_iter().next().unwrap()
+    }
+
     #[test]
     fn automation_ops_round_trip() {
         let mut app = OpenCADStudio::new_for_test();

src/app/command_driver.rs

@@ -566,6 +566,7 @@ impl OpenCADStudio {
     /// the headless automation feeder so both behave identically.
     pub(super) fn run_command_line(&mut self, cmd: &str) -> Task<Message> {
         let i = self.active_tab;
+        self.command_line.unconsumed.clear();
         let tokens: Vec<&str> = cmd.split_whitespace().collect();
         if tokens.len() <= 1 {
             return self.dispatch_command(cmd);
@@ -597,18 +598,64 @@ impl OpenCADStudio {
     /// Feed `tokens[1..]` to the active interactive command as points / option
     /// keywords, then terminate it as if Enter were pressed. No-op when no
     /// command is active.
+    ///
+    /// Two things happen when the tokens run out mid-way, both needed so a
+    /// headless caller can tell what actually happened:
+    ///
+    /// * If the in-place text editor took over (the `TEXT` content step — the
+    ///   command itself has already ended by then, `active_cmd` is `None`), the
+    ///   remaining tokens are that text: they are typed into the editor and
+    ///   committed, the same messages the control surface's `text_input` /
+    ///   `text_commit` actions dispatch. Without this the tail of the line was
+    ///   silently dropped and `TEXT 0,0 5 0 hi` created nothing at all.
+    /// * Anything still left over was never claimed by any prompt; it is
+    ///   recorded in [`CommandLine::unconsumed`] instead of vanishing.
     pub(super) fn finish_active_command(&mut self, tokens: &[String]) -> Task<Message> {
         let i = self.active_tab;
         if self.tabs[i].active_cmd.is_none() {
             return Task::none();
         }
         self.last_point = None;
+        // An editor that is *already* open belongs to someone else (an earlier
+        // line that stopped at the content step). Only the editor this line
+        // opens by feeding its own tokens may be handed the tail.
+        let editor_was_open = self.text_inline.is_some();
         let mut tasks = Vec::new();
+        // First token is the command verb itself; prompts start consuming after it.
+        let mut consumed = 1;
         for tok in &tokens[1..] {
             if self.tabs[i].active_cmd.is_none() {
                 break;
             }
             tasks.push(self.feed_active_cmd(tok));
+            consumed += 1;
+        }
+        if !editor_was_open && self.text_inline.is_some() && consumed < tokens.len() {
+            // Fill the editor the same way `Message::TextInlineInput` does, then
+            // commit *synchronously* so the outcome is observable: dispatching
+            // `TextInlineOk` as a task would hide whether the entity was really
+            // created, and the tail token would be counted as consumed either
+            // way — the caller would have no way to tell "text created" from
+            // "text silently dropped".
+            if let Some(editor) = self.text_inline.as_mut() {
+                editor.value = tokens[consumed..].join(" ");
+            }
+            let committed = self.text_inline_commit();
+            tasks.push(self.post_editor_closed(committed));
+            if committed {
+                consumed = tokens.len();
+                // `TEXT` repeats by design: on a committed string it re-arms for
+                // the next line (`open_next_line`), which re-opens the editor. A
+                // batch line is a complete unit, and leaving that repeat armed
+                // would make the *next* line get eaten as the next text position,
+                // so end the command here, exactly as Esc does in the GUI.
+                tasks.push(Task::done(Message::CommandEscape));
+            }
+            // On a failed commit the tokens stay unconsumed (assigned below), so
+            // the caller still sees the text it passed in.
+        }
+        if consumed < tokens.len() {
+            self.command_line.unconsumed = tokens[consumed..].to_vec();
         }
         tasks.push(self.feed_command(StepInput::Enter));
         Task::batch(tasks)

src/ui/command_line.rs

@@ -129,6 +129,13 @@ pub struct CommandLine {
     pub history: Vec<HistoryEntry>,
     pub error_revision: u64,
     pub last_error: Option<String>,
+    /// Tokens from the last multi-token command line that no prompt ever
+    /// consumed — e.g. a trailing word after an in-place text step, or a typo'd
+    /// extra argument (`CIRCLE 0,0 5 9`). The headless automation feeder reports
+    /// them so a caller can tell "part of my line was silently dropped" from
+    /// "the command ran exactly as typed". Cleared at the start of every
+    /// `run_command_line`, not persisted.
+    pub unconsumed: Vec<String>,
     /// Successfully dispatched commands used for ↑/↓ recall, newest last.
     /// Stored separately because recall also maintains its own cursor and draft.
     pub cmd_recall: Vec<String>,
@@ -186,6 +193,7 @@ impl Default for CommandLine {
             history: Vec::new(),
             error_revision: 0,
             last_error: None,
+            unconsumed: Vec::new(),
             cmd_recall: Vec::new(),
             recent_commands: Vec::new(),
             recent_inputs: Vec::new(),

src/ui/style/plotstyle.rs

@@ -84,6 +84,35 @@ fn field_style(theme: &Theme, status: text_input::Status) -> text_input::Style {
     }
 }
 
+/// Layer names bucketed by ACI (AutoCAD Color Index), index = ACI.
+///
+/// Pure function of `document.layers`: one entry per layer whose color is
+/// `Color::Index(aci)` with `1 <= aci < 256`, each bucket sorted
+/// case-insensitively. Extracted from `view_window` so the table can be
+/// unit-tested and benched without building widgets. `pub` (not
+/// `pub(crate)`) so the `cargo bench` harness (`benches/`, an external
+/// crate) can measure it as `ui_plotstyle_layer_usage`.
+pub fn build_layer_usage(document: &acadrust::CadDocument) -> Vec<Vec<String>> {
+    let mut layer_usage = vec![Vec::<String>::new(); 256];
+
+    for layer in document.layers.iter() {
+        if let acadrust::types::Color::Index(aci) = &layer.color {
+            let index = *aci as usize;
+
+            if index > 0 && index < layer_usage.len() {
+                layer_usage[index].push(layer.name.clone());
+            }
+        }
+    }
+
+    for layers in &mut layer_usage {
+        if layers.len() > 1 {
+            layers.sort_by_cached_key(|name| name.to_ascii_lowercase());
+        }
+    }
+    layer_usage
+}
+
 pub fn view_window<'a>(
     document: &'a acadrust::CadDocument,
     table: Option<&'a crate::io::plot_style::PlotStyleTable>,
@@ -95,23 +124,11 @@ pub fn view_window<'a>(
 ) -> Element<'a, Message> {
     let show_layer_usage = !table.is_some_and(|table| table.is_stb);
 
-let mut layer_usage = vec![Vec::<String>::new(); 256];
-
-    if show_layer_usage {
-        for layer in document.layers.iter() {
-            if let acadrust::types::Color::Index(aci) = &layer.color {
-                let index = *aci as usize;
-
-                if index > 0 && index < layer_usage.len() {
-                    layer_usage[index].push(layer.name.clone());
-                }
-            }
-        }
-
-        for layers in &mut layer_usage {
-            layers.sort_by_key(|name| name.to_ascii_lowercase());
-        }
-    }
+    let layer_usage = if show_layer_usage {
+        build_layer_usage(document)
+    } else {
+        Vec::new()
+    };
     let table_name = table
         .map(|t| t.name.clone())
         .unwrap_or_else(|| t!("(no table loaded)").into_owned());
@@ -536,3 +553,58 @@ let mut layer_usage = vec![Vec::<String>::new(); 256];
         .height(sizing.height)
         .into()
 }
+
+#[cfg(test)]
+mod layer_usage_tests {
+    use super::*;
+
+    fn indexed_layer(
+        doc: &mut acadrust::CadDocument,
+        name: &str,
+        aci: u8,
+    ) -> acadrust::tables::Layer {
+        let mut layer = acadrust::tables::Layer::new(name);
+        layer.handle = doc.allocate_handle();
+        layer.color = acadrust::types::Color::Index(aci);
+        layer
+    }
+
+    #[test]
+    fn build_layer_usage_buckets_index_colors() {
+        let mut doc = acadrust::CadDocument::new();
+        let a = indexed_layer(&mut doc, "WALLS", 1);
+        let b = indexed_layer(&mut doc, "doors", 1);
+        let c = indexed_layer(&mut doc, "ROOF", 3);
+        let _ = doc.layers.add(a);
+        let _ = doc.layers.add(b);
+        let _ = doc.layers.add(c);
+
+        let usage = build_layer_usage(&doc);
+
+        assert_eq!(usage.len(), 256);
+        assert_eq!(usage[1], vec!["doors".to_string(), "WALLS".to_string()]);
+        assert_eq!(usage[3], vec!["ROOF".to_string()]);
+        assert!(usage[2].is_empty());
+    }
+
+    #[test]
+    fn build_layer_usage_ignores_non_index_colors() {
+        let mut doc = acadrust::CadDocument::new();
+        let mut truecolor = acadrust::tables::Layer::new("TRUECOLOR");
+        truecolor.handle = doc.allocate_handle();
+        truecolor.color = acadrust::types::Color::Rgb {
+            r: 10,
+            g: 20,
+            b: 30,
+        };
+        let _ = doc.layers.add(truecolor);
+
+        let usage = build_layer_usage(&doc);
+
+        assert!(
+            usage.iter().all(|bucket| !bucket.iter().any(|n| n == "TRUECOLOR")),
+            "truecolor layer must not appear in any ACI bucket"
+        );
+    }
+
+}