diff --git a/crates/tinytools-std/src/network/fixtures/web_fetch.json b/crates/tinytools-std/src/network/fixtures/web_fetch.json index 18a24c8..a715af6 100644 --- a/crates/tinytools-std/src/network/fixtures/web_fetch.json +++ b/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" }, diff --git a/crates/tinytools-std/src/network/web_fetch.rs b/crates/tinytools-std/src/network/web_fetch.rs index 1775e0b..9fba8a5 100644 --- a/crates/tinytools-std/src/network/web_fetch.rs +++ b/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 ``. 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() { diff --git a/crates/tinytools-std/src/network/web_fetch_tests.rs b/crates/tinytools-std/src/network/web_fetch_tests.rs index 239b5e9..22327df 100644 --- a/crates/tinytools-std/src/network/web_fetch_tests.rs +++ b/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\ +

the prose that matters

", + "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 `` — 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}"); +}