Apply web_fetch's max_bytes to the extracted text, not the markup

crates/tinytools-std/src/network/fixtures/web_fetch.json

@@ -6,7 +6,7 @@
   "schema": {
     "properties": {
       "max_bytes": {
-        "description": "Truncate body at this many bytes (default 1_000_000).",
+        "description": "Cap the returned text at this many bytes (default 1_000_000). Bounds the OUTPUT — the extracted markdown, or the raw body with raw:true — never the markup the extractor reads, so lowering it cannot cost you content the page actually had.",
         "minimum": 1,
         "type": "integer"
       },

crates/tinytools-std/src/network/web_fetch.rs

@@ -127,7 +127,11 @@ impl Tool for WebFetchTool {
                 "url": { "type": "string", "description": "Absolute http(s) URL." },
                 "max_bytes": {
                     "type": "integer",
-                    "description": "Truncate body at this many bytes (default 1_000_000).",
+                    "description": "Cap the returned text at this many bytes \
+                     (default 1_000_000). Bounds the OUTPUT — the extracted \
+                     markdown, or the raw body with raw:true — never the markup \
+                     the extractor reads, so lowering it cannot cost you content \
+                     the page actually had.",
                     "minimum": 1
                 },
                 "raw": {
@@ -305,12 +309,6 @@ impl WebFetchTool {
         }
 
         let downloaded = body.len();
-        let (body, byte_capped) = if downloaded > max_bytes {
-            let cut = floor_char_boundary(&body, max_bytes);
-            (body[..cut].to_string(), true)
-        } else {
-            (body, false)
-        };
 
         // Markdown by default. A page's prose is a small fraction of its
         // bytes; handing the raw document to the model (and to the payload
@@ -319,24 +317,26 @@ impl WebFetchTool {
         // content transforms.
         let converted =
             !raw_requested && is_html(self.html.as_ref(), &body, content_type.as_deref());
-        let content = if converted {
-            self.html.to_markdown(&body)
-        } else {
-            body
-        };
+        let rendered = render_body(self.html.as_ref(), body, converted, max_bytes);
 
-        let extracted = content.len();
+        let extracted = rendered.extracted;
         let mut header = format!("status={} url={final_url}", status.as_u16());
         if converted {
             header.push_str(" content=markdown");
         }
-        if byte_capped {
-            header.push_str(&format!(" download_capped_at={max_bytes}B"));
+        // `output_capped_at`, not the old `download_capped_at`: nothing here
+        // ever capped a download — `resp.text()` above materialises the whole
+        // body regardless — and naming it that sent a reader looking in the
+        // wrong place for the content that went missing.
+        if rendered.output_capped {
+            header.push_str(&format!(" output_capped_at={max_bytes}B"));
         }
+        append_markup_truncation_header(&mut header, &rendered);
         if converted && extracted < downloaded {
             header.push_str(&format!(" extracted={extracted}B_of_{downloaded}B"));
         }
         header.push('\n');
+        let content = rendered.content;
 
         // Full extracted content. Bounding it — the head/tail window, the
         // spill to an artifact and the paging handle — belongs to
@@ -412,6 +412,87 @@ fn error_body_excerpt(
     }
 }
 
+/// Raw markup handed to the HTML extractor, at most.
+///
+/// Not a byte budget — the caller's `max_bytes` owns that, applied to the
+/// output. This exists only so a pathological document cannot cost unbounded
+/// CPU in `to_markdown`, a cost the old pre-truncation ordering hid by
+/// accident. 8 MiB is ~19x the largest real page measured here (a 434 KB
+/// client-rendered spreadsheet) and ~8x the default `max_response_size`, so no
+/// realistic page reaches it.
+const EXTRACTOR_INPUT_CEILING: usize = 8 * 1024 * 1024;
+
+/// A body turned into what the caller reads.
+struct RenderedBody {
+    /// The text to return, bounded by the caller's `max_bytes`.
+    content: String,
+    /// Length *before* that bound, so the header's ratio describes the
+    /// extraction rather than the truncation.
+    extracted: usize,
+    /// Whether `content` was cut to fit `max_bytes`.
+    output_capped: bool,
+    /// Whether the markup was cut before the extractor saw it, which only
+    /// happens past [`EXTRACTOR_INPUT_CEILING`].
+    markup_truncated: bool,
+}
+
+/// Add the extractor input ceiling to a fetch header when markup was cut.
+fn append_markup_truncation_header(header: &mut String, rendered: &RenderedBody) {
+    if rendered.markup_truncated {
+        header.push_str(&format!(" markup_truncated_at={EXTRACTOR_INPUT_CEILING}B"));
+    }
+}
+
+/// Convert, **then** bound.
+///
+/// The order is the whole of this function. It used to be the other way round,
+/// and the cap was therefore destroying the thing it was meant to measure: a
+/// 432,864-byte client-rendered page fetched with `max_bytes: 50000` was cut at
+/// byte 50,000 — mid-tag, mid-DOM — and the readability pass, handed that
+/// wreckage, recovered only the `<title>`. The result was
+/// `extracted=48B_of_432864B`, reported as `status=200`. The same URL with no
+/// `max_bytes` yields `extracted=37243B_of_433638B`: the real document.
+/// Identical tool, identical extractor, 776x the content, one parameter.
+///
+/// So a caller setting a sensible cost bound silently lost the page, and the
+/// knob it would then reach for — raising the cap — was the right knob turned
+/// too timidly, which is the worst case for learning anything from the failure.
+///
+/// Bounding the output is also what the schema promises, and it costs no
+/// memory: the caller has already materialised the whole body. Only the
+/// extractor's input needs a ceiling, and that is for CPU.
+fn render_body(
+    html: &dyn HtmlExtractor,
+    body: String,
+    converted: bool,
+    max_bytes: usize,
+) -> RenderedBody {
+    let (markup_truncated, body) = if converted && body.len() > EXTRACTOR_INPUT_CEILING {
+        let cut = floor_char_boundary(&body, EXTRACTOR_INPUT_CEILING);
+        (true, body[..cut].to_string())
+    } else {
+        (false, body)
+    };
+    let full = if converted {
+        html.to_markdown(&body)
+    } else {
+        body
+    };
+    let extracted = full.len();
+    let (content, output_capped) = if extracted > max_bytes {
+        let cut = floor_char_boundary(&full, max_bytes);
+        (full[..cut].to_string(), true)
+    } else {
+        (full, false)
+    };
+    RenderedBody {
+        content,
+        extracted,
+        output_capped,
+        markup_truncated,
+    }
+}
+
 /// The largest index at or below `index` that is a char boundary of `s`.
 fn floor_char_boundary(s: &str, index: usize) -> usize {
     if index >= s.len() {

crates/tinytools-std/src/network/web_fetch_tests.rs

@@ -378,3 +378,127 @@ async fn a_redirect_is_still_reported_as_a_successful_result() {
     assert!(result.output().contains("status=301"));
     assert!(result.output().contains("location=https://example.com/new"));
 }
+
+// --- The cap bounds the output, not the extractor's input ------------------
+//
+// `TestHtml::to_markdown` strips tags, so a document whose prose sits past the
+// cap offset is the shape that discriminates: cutting the markup first loses
+// the prose outright, cutting the rendered text keeps it.
+
+/// Markup whose readable text sits behind `filler` bytes of attribute.
+fn page_with_prose_after(filler: usize) -> String {
+    format!(
+        "<!DOCTYPE html><html><head><title>T</title></head><body>\
+         <div data-pad=\"{}\"></div><p>the prose that matters</p></body></html>",
+        "f".repeat(filler)
+    )
+}
+
+#[test]
+fn the_cap_applies_to_the_extracted_text_not_the_markup() {
+    // The regression this function exists for. A 432,864-byte page fetched at
+    // `max_bytes: 50000` used to come back as `extracted=48B_of_432864B` — the
+    // whole document reduced to its `<title>` — because the markup was cut
+    // mid-DOM before the extractor ever saw it.
+    let body = page_with_prose_after(4_000);
+    assert!(body.len() > 1_000, "the prose must sit past the cap");
+
+    let rendered = render_body(&TestHtml, body, true, 1_000);
+    assert!(
+        rendered.content.contains("the prose that matters"),
+        "cutting the markup first would have lost this: {:?}",
+        rendered.content
+    );
+}
+
+#[test]
+fn cutting_the_markup_first_really_does_destroy_the_extraction() {
+    // The counterfactual, so the test above cannot pass vacuously: the old
+    // ordering, performed by hand, loses the prose from the same document.
+    let body = page_with_prose_after(4_000);
+    let truncated = &body[..1_000];
+    assert!(
+        !TestHtml
+            .to_markdown(truncated)
+            .contains("the prose that matters"),
+        "a pre-extraction cut is what destroyed the content"
+    );
+}
+
+#[test]
+fn the_reported_length_is_the_extraction_not_the_truncation() {
+    // `extracted` is pre-cap on purpose: the header's `extracted=XB_of_YB`
+    // ratio describes how much of the document the extractor found, which is
+    // the number that reveals a collapse. Measuring post-cap would report the
+    // cap back to the caller as if it were the page.
+    let body = page_with_prose_after(4_000);
+    let full = TestHtml.to_markdown(&body);
+    let rendered = render_body(&TestHtml, body, true, 4);
+    assert_eq!(rendered.extracted, full.len());
+    assert!(rendered.output_capped);
+    assert!(rendered.content.len() <= 4);
+}
+
+#[test]
+fn an_output_within_the_cap_is_returned_whole_and_unflagged() {
+    let body = page_with_prose_after(16);
+    let rendered = render_body(&TestHtml, body, true, 1_000_000);
+    assert!(!rendered.output_capped);
+    assert!(!rendered.markup_truncated);
+    assert!(rendered.content.contains("the prose that matters"));
+}
+
+#[test]
+fn markup_truncated_input_is_reported_in_the_fetch_header() {
+    // Drive the body across the extractor's independent input ceiling. The
+    // large attribute keeps extracted text small, while proving that the
+    // extractor input itself was bounded and reported to the caller.
+    let body = format!(
+        "<!DOCTYPE html><html><body><div data-pad=\"{}\"></div><p>visible</p></body></html>",
+        "x".repeat(EXTRACTOR_INPUT_CEILING)
+    );
+    assert!(body.len() > EXTRACTOR_INPUT_CEILING);
+    let rendered = render_body(&TestHtml, body, true, 1_000);
+    assert!(rendered.markup_truncated);
+    let mut output = "status=200 url=https://example.com content=markdown".to_string();
+    append_markup_truncation_header(&mut output, &rendered);
+    assert!(
+        output.contains(&format!("markup_truncated_at={EXTRACTOR_INPUT_CEILING}B")),
+        "header should disclose the extractor input ceiling: {output}"
+    );
+}
+
+#[test]
+fn raw_output_is_bounded_by_the_same_cap() {
+    // With `raw: true` there is no extraction, so the cap applies to the body
+    // itself — the one case where cutting the input and cutting the output are
+    // the same act.
+    let rendered = render_body(&TestHtml, "abcdefghij".to_string(), false, 4);
+    assert_eq!(rendered.content, "abcd");
+    assert!(rendered.output_capped);
+    assert_eq!(rendered.extracted, 10);
+}
+
+#[test]
+fn the_markup_ceiling_is_far_above_any_real_page() {
+    // Sized against the measured worst case, not picked round. A ceiling near
+    // the old default would reintroduce the bug for ordinary documents.
+    const {
+        assert!(EXTRACTOR_INPUT_CEILING >= 8 * 1024 * 1024);
+        assert!(EXTRACTOR_INPUT_CEILING > 433_638 * 10);
+    }
+}
+
+#[test]
+fn the_schema_says_which_side_of_the_extractor_it_bounds() {
+    // The description is a caller's only account of what the knob does, and
+    // the old wording ("Truncate body at this many bytes") is what made
+    // lowering it look free.
+    let tool = fetch(test_security(), vec![], None, None);
+    let schema = tool.parameters_schema();
+    let desc = schema["properties"]["max_bytes"]["description"]
+        .as_str()
+        .expect("max_bytes documents itself");
+    assert!(desc.contains("OUTPUT"), "{desc}");
+    assert!(desc.contains("never the markup"), "{desc}");
+}