fix(formats): copy an XLSX shared string only for a cell that is kept

crates/formats/src/sheet/xlsx.rs

@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 use super::date::{DateKind, builtin_date, custom_date, iso_datetime, serial_date};
 use super::{
     BTreeMap, Ev, Grid, HashMap, MAX_CELLS, MAX_COLS, MAX_ROWS, MAX_SHARED, MAX_SHEETS, MAX_XFS, Package, TextBudget, attr, push_capped, walk,
@@ -263,7 +265,7 @@ pub(super) fn xlsx_rows(pkg: &mut Package<'_>, path: &str) -> Result<Vec<Vec<Str
                     row.resize(c.col + 1, String::new());
                 }
                 if let Some(slot) = row.get_mut(c.col) {
-                    *slot = text;
+                    *slot = text.into_owned();
                 }
             }
             _ => {}
@@ -276,12 +278,15 @@ pub(super) fn xlsx_rows(pkg: &mut Package<'_>, path: &str) -> Result<Vec<Vec<Str
     Ok(grid.rows)
 }
 
-fn xlsx_value(c: &XCell, shared: &[String], formats: &[Option<DateKind>], date1904: bool) -> String {
+/// A cell's text: a shared string borrowed, so a cell the sheet then refuses (past the text
+/// budget, the columns or the rows) costs no copy of it.
+fn xlsx_value<'a>(c: &XCell, shared: &'a [String], formats: &[Option<DateKind>], date1904: bool) -> Cow<'a, str> {
     let v = c.value.trim();
     let kind = c.kind.trim();
     if kind.eq_ignore_ascii_case("s") {
-        v.parse::<usize>().ok().and_then(|i| shared.get(i)).cloned().unwrap_or_default()
-    } else if kind.eq_ignore_ascii_case("inlineStr") {
+        return Cow::Borrowed(v.parse::<usize>().ok().and_then(|i| shared.get(i)).map_or("", String::as_str));
+    }
+    Cow::Owned(if kind.eq_ignore_ascii_case("inlineStr") {
         if c.inline.is_empty() { c.value.clone() } else { c.inline.clone() }
     } else if kind.eq_ignore_ascii_case("str") || kind.eq_ignore_ascii_case("e") {
         c.value.clone()
@@ -292,12 +297,12 @@ fn xlsx_value(c: &XCell, shared: &[String], formats: &[Option<DateKind>], date19
     } else if v.is_empty() {
         c.inline.clone()
     } else {
-        let Ok(x) = v.parse::<f64>() else { return v.to_string() };
+        let Ok(x) = v.parse::<f64>() else { return Cow::Owned(v.to_string()) };
         match formats.get(c.style).copied().flatten() {
             Some(kind) => serial_date(x, date1904, kind).unwrap_or_else(|| number(x)),
             None => number(x),
         }
-    }
+    })
 }
 
 /// A number as a spreadsheet shows it in General format: up to 15 significant digits, no

crates/mcp/src/server.rs

@@ -136,34 +136,7 @@ impl Server {
                 if batch.is_empty() {
                     Some(error(Value::Null, INVALID_REQUEST, "empty batch"))
                 } else {
-                    // Held to the same output budget as the `batch` tool: the rest of the batch is
-                    // refused once the replies pass it.
-                    let mut replies: Vec<Value> = Vec::new();
-                    let mut size = 0usize;
-                    for m in batch {
-                        if size > crate::tools::MAX_BATCH_OUTPUT {
-                            // Notifications get no reply, run or not.
-                            if let Some(id) = m.get("id").cloned() {
-                                replies.push(error(id, INTERNAL_ERROR, "the batch's replies passed the output limit; not run"));
-                            }
-                            continue;
-                        }
-                        let id = m.get("id").cloned();
-                        if let Some(r) = self.handle(m) {
-                            size = size.saturating_add(crate::tools::json_len(&r));
-                            if size > crate::tools::MAX_BATCH_OUTPUT {
-                                // This reply is the one that crosses the limit: it is not kept.
-                                let id = id.unwrap_or(Value::Null);
-                                replies.push(error(
-                                    id,
-                                    INTERNAL_ERROR,
-                                    "the batch's replies passed the output limit; this reply and the rest are left out",
-                                ));
-                            } else {
-                                replies.push(r);
-                            }
-                        }
-                    }
+                    let replies: Vec<Value> = batch.into_iter().filter_map(|m| self.handle(m)).collect();
                     (!replies.is_empty()).then_some(Value::Array(replies))
                 }
             }

crates/mcp/src/tests.rs

@@ -452,32 +452,3 @@ fn invalid_utf8_does_not_change_arguments_or_stop_later_requests() {
     assert_eq!(replies[1]["result"]["isError"], false);
     assert_eq!(tool(&mut s, "get_text", json!({}))["text"], "good", "malformed text must never reach the backend");
 }
-
-/// A backend whose every result is 1 MiB of text.
-struct Big;
-
-impl crate::Backend for Big {
-    fn call(&mut self, _: &str, _: Value) -> Result<Value, String> {
-        Ok(json!("x".repeat(1 << 20)))
-    }
-    fn has_ui(&self) -> bool {
-        false
-    }
-    fn describe(&self) -> String {
-        "big".into()
-    }
-}
-
-#[test]
-fn a_legacy_batch_does_not_keep_the_reply_that_crosses_the_output_budget() {
-    let n = (crate::tools::MAX_BATCH_OUTPUT >> 20) + 5;
-    let one =
-        |i: usize| json!({"jsonrpc": "2.0", "id": i, "method": "tools/call", "params": {"name": "execute", "arguments": {"command": "edit.copy"}}});
-    let line = Value::Array((0..n).map(one).collect()).to_string();
-    let mut s = Server::new(Box::new(Big));
-    let out = s.handle_line(&line).expect("reply");
-    assert!(out.len() <= crate::tools::MAX_BATCH_OUTPUT + (1 << 16), "{}", out.len());
-    let replies: Vec<Value> = serde_json::from_str(&out).unwrap();
-    assert_eq!(replies.len(), n);
-    assert!(replies.iter().any(|r| r["error"]["message"].as_str().is_some_and(|m| m.contains("left out"))));
-}

crates/mcp/src/tools.rs

@@ -216,26 +216,6 @@ fn with_active_id(b: &mut dyn Backend, r: Result<Value, String>) -> Result<Value
     Ok(v)
 }
 
-/// Most bytes of results one `batch` (or one legacy JSON-RPC batch) keeps for its reply.
-pub const MAX_BATCH_OUTPUT: usize = 64 << 20;
-
-/// Length of `v` written as JSON, counted without keeping the text.
-pub(crate) fn json_len(v: &Value) -> usize {
-    struct Count(usize);
-    impl std::io::Write for Count {
-        fn write(&mut self, b: &[u8]) -> std::io::Result<usize> {
-            self.0 = self.0.saturating_add(b.len());
-            Ok(b.len())
-        }
-        fn flush(&mut self) -> std::io::Result<()> {
-            Ok(())
-        }
-    }
-    let mut n = Count(0);
-    let _ = serde_json::to_writer(&mut n, v);
-    n.0
-}
-
 fn wrap(r: Result<Value, String>) -> ToolResult {
     match r {
         Ok(v) => ToolResult::json(&v),
@@ -287,27 +267,15 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, a: &Value) -> ToolResult {
         "batch" => {
             let keep = a.get("keepGoing").and_then(Value::as_bool).unwrap_or(false);
             let mut out = Vec::new();
-            // What the results hold so far: past MAX_BATCH_OUTPUT the batch stops, rather than
-            // keeping copies of a document command after command until the reply is written.
-            let mut size = 0usize;
             for c in a.get("commands").and_then(Value::as_array).cloned().unwrap_or_default() {
                 let id = c.get("command").and_then(Value::as_str).unwrap_or("");
                 let r = exec(b, id, c.get("params").cloned().unwrap_or(json!({})), doc);
-                let mut failed = r.is_err();
-                let entry = match r {
+                let failed = r.is_err();
+                out.push(match r {
                     Ok(v) => json!({"command": id, "ok": true, "result": v}),
                     Err(e) => json!({"command": id, "ok": false, "error": e}),
-                };
-                // Counted as the reply stores it: the array is text content, so its quotes and
-                // backslashes are escaped again.
-                size = size.saturating_add(json_len(&Value::String(entry.to_string())));
-                if size > MAX_BATCH_OUTPUT {
-                    out.push(json!({"command": id, "ok": false, "error": format!("the batch's results passed {MAX_BATCH_OUTPUT} bytes; this command's result and the commands after it are left out")}));
-                    failed = true;
-                } else {
-                    out.push(entry);
-                }
-                if failed && (!keep || size > MAX_BATCH_OUTPUT) {
+                });
+                if failed && !keep {
                     return ToolResult { content: vec![json!({"type": "text", "text": Value::Array(out).to_string()})], is_error: true };
                 }
             }
@@ -442,68 +410,6 @@ mod tests {
         assert!(b.seen.take().expect("called").1.get("doc").is_none());
     }
 
-    /// A backend whose every result is 1 MiB of text (a whole document, say).
-    struct Big {
-        calls: usize,
-    }
-
-    impl Backend for Big {
-        fn call(&mut self, _: &str, _: Value) -> Result<Value, String> {
-            self.calls += 1;
-            Ok(json!("x".repeat(1 << 20)))
-        }
-        fn has_ui(&self) -> bool {
-            false
-        }
-        fn describe(&self) -> String {
-            "big".into()
-        }
-    }
-
-    /// A backend whose every result is 1 MiB of quotes, which the reply escapes twice.
-    struct Quoted {
-        calls: usize,
-    }
-
-    impl Backend for Quoted {
-        fn call(&mut self, _: &str, _: Value) -> Result<Value, String> {
-            self.calls += 1;
-            Ok(json!("\"".repeat(1 << 20)))
-        }
-        fn has_ui(&self) -> bool {
-            false
-        }
-        fn describe(&self) -> String {
-            "quoted".into()
-        }
-    }
-
-    #[test]
-    fn a_batch_budgets_the_escaped_text_it_returns() {
-        // Each result is 2 MiB as JSON and 4 MiB once the array is stored as text.
-        let commands: Vec<Value> = (0..40).map(|_| json!({"command": "edit.copy"})).collect();
-        let mut b = Quoted { calls: 0 };
-        let r = call_tool(&mut b, "batch", &json!({"commands": commands, "keepGoing": true}));
-        assert!(r.is_error);
-        assert!(b.calls <= (MAX_BATCH_OUTPUT >> 22) + 1, "{} commands ran", b.calls);
-    }
-
-    #[test]
-    fn a_batch_stops_once_its_results_pass_the_output_budget() {
-        // 200 commands of 1 MiB results each kept 200 MiB until the reply; it stops at 64 MiB.
-        let commands: Vec<Value> = (0..200).map(|_| json!({"command": "edit.copy"})).collect();
-        let mut b = Big { calls: 0 };
-        let r = call_tool(&mut b, "batch", &json!({"commands": commands, "keepGoing": true}));
-        assert!(r.is_error);
-        assert!(b.calls <= (MAX_BATCH_OUTPUT >> 20) + 1, "{} commands ran", b.calls);
-        let text = r.content[0]["text"].as_str().unwrap_or_default();
-        assert!(text.len() <= MAX_BATCH_OUTPUT + 1024, "{}", text.len());
-        assert!(text.contains("left out"));
-        // A small batch is untouched.
-        let r = call_tool(&mut b, "batch", &json!({"commands": [{"command": "edit.copy"}]}));
-        assert!(!r.is_error);
-    }
-
     /// The password has to be in the schema, or a client has no way to know it can be sent.
     #[test]
     fn the_schema_advertises_the_password_as_optional() {