Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 58 additions & 6 deletions crates/tinysearch-bus/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,58 @@ fn result_images(response: &ExecuteToolResponse) -> Vec<Option<&str>> {
.collect()
}

/// A URL as compared for repeats: the scheme and host of an HTTP(S) URL are
/// case-insensitive, the rest (including any userinfo) is not.
fn url_key(url: &str) -> String {
let url = url.trim();
let Some(split) = url.find("://") else {
return url.to_string();
};
let (scheme, rest) = url.split_at(split);
if !scheme.eq_ignore_ascii_case("http") && !scheme.eq_ignore_ascii_case("https") {
return url.to_string();
}
let rest = &rest[3..];
let end = rest.find(['/', '?', '#']).unwrap_or(rest.len());
let (authority, tail) = rest.split_at(end);
let (userinfo, host) = match authority.rfind('@') {
Some(at) => authority.split_at(at + 1),
None => ("", authority),
};
format!(
"{}://{userinfo}{}{tail}",
scheme.to_ascii_lowercase(),
host.to_ascii_lowercase()
)
}

/// The answer text to print.
///
/// A page-contents response may carry the first page as its answer as well as
/// in its result snippet. Under non-default options that copy is left out, but
/// only when it is a confirmed copy of a page that is actually listed (one
/// text is a prefix of the other); any other contents answer is kept, clipped
/// to the per-page budget. Answer-role responses are never touched, and the
/// default options keep the original output.
fn answer_text(
response: &ExecuteToolResponse,
max_results: usize,
options: &RenderOptions,
) -> Option<String> {
let answer = non_empty(response.answer.as_deref())?;
if response.role != Some(Role::Contents) || *options == RenderOptions::default() {
return Some(answer.to_string());
}
let answer_trimmed = answer.trim();
let copies_listed_page = response.results.iter().take(max_results).any(|result| {
non_empty(result.snippet.as_deref()).is_some_and(|snippet| {
let snippet = snippet.trim();
answer_trimmed.starts_with(snippet) || snippet.starts_with(answer_trimmed)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve contents beyond a shorter matching snippet

For a TinyFish Role::Contents response with a 1,300-character first page, normalize stores 1,200 characters in the snippet while answer_for("tinyfish_fetch", ...) retains all 1,300. The new starts_with test treats the shorter snippet as a complete duplicate and suppresses the answer, so both compact renderers silently discard the final 100 characters even though the whole page fits their 1,500-character budget. Compare the text that will actually be displayed and render the longer budgeted copy rather than discarding unique answer content.

Useful? React with 👍 / 👎.

})
});
(!copies_listed_page).then(|| truncate(answer, options.contents_chars, "…"))
}

/// The citations to list under `Sources`, with their 1-based numbers.
///
/// All of them unless `options.compact_sources` is set; then only those that
Expand All @@ -134,14 +186,14 @@ fn sources<'a>(
if !options.compact_sources {
return numbered.collect();
}
let shown: HashSet<&str> = response
let shown: HashSet<String> = response
.results
.iter()
.take(max_results)
.map(|result| result.url.trim())
.map(|result| url_key(&result.url))
.collect();
numbered
.filter(|(_, citation)| !shown.contains(citation.url.trim()))
.filter(|(_, citation)| !shown.contains(&url_key(&citation.url)))
.take(max_results)
.collect()
}
Expand All @@ -162,9 +214,9 @@ pub fn render_text_with(
) -> String {
let excerpt_chars = options.excerpt_for(response.role);
let mut lines = vec![heading(response, subject)];
if let Some(answer) = non_empty(response.answer.as_deref()) {
if let Some(answer) = answer_text(response, max_results, options) {
lines.push(String::new());
lines.push(answer.to_string());
lines.push(answer);
}
let images = result_images(response);
for (index, item) in response.results.iter().take(max_results).enumerate() {
Expand Down Expand Up @@ -219,7 +271,7 @@ pub fn render_markdown_with(
) -> String {
let excerpt_chars = options.excerpt_for(response.role);
let mut out = format!("# {}\n", heading(response, subject));
if let Some(answer) = non_empty(response.answer.as_deref()) {
if let Some(answer) = answer_text(response, max_results, options) {
let _ = write!(out, "\n{answer}\n");
}
let images = result_images(response);
Expand Down
102 changes: 102 additions & 0 deletions crates/tinysearch-bus/src/render/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,108 @@ fn contents_pages_are_bounded_by_the_contents_cap() {
assert!(!text.contains(&"y".repeat(201)));
}

#[test]
fn compact_matches_repeated_sources_by_case_insensitive_host_only() {
let mut r = listed(1);
r.results[0].url = "https://Example.COM/Path".into();
r.citations = vec![
Citation {
url: "HTTPS://example.com/Path".into(),
title: None,
},
Citation {
url: "https://example.com/path".into(),
title: None,
},
];
let text = render_text_with(&r, "q", 5, &RenderOptions::compact());
assert!(!text.contains("[1]"));
assert!(text.contains("[2] https://example.com/path"));
}

#[test]
fn compact_keeps_a_citation_whose_userinfo_differs_only_in_case() {
let mut r = listed(1);
r.results[0].url = "https://reader:TokenA@Example.com/Path".into();
r.citations = vec![
Citation {
url: "https://reader:TokenA@example.com/Path".into(),
title: None,
},
Citation {
url: "https://reader:Tokena@example.com/Path".into(),
title: None,
},
];
let text = render_text_with(&r, "q", 5, &RenderOptions::compact());
assert!(!text.contains("[1]"));
assert!(text.contains("[2] https://reader:Tokena@example.com/Path"));
}

#[test]
fn contents_answer_is_kept_when_no_snippet_duplicates_it() {
let mut page = response(Some(Role::Contents));
page.results[0].snippet = None;
page.answer = Some("z".repeat(4000));
let narrow = RenderOptions {
contents_chars: 200,
..RenderOptions::default()
};
for options in [RenderOptions::compact(), narrow] {
for out in [
render_text_with(&page, "u", 5, &options),
render_markdown_with(&page, "u", 5, &options),
] {
assert!(out.contains(&"z".repeat(options.contents_chars.min(200))));
assert!(!out.contains(&"z".repeat(options.contents_chars + 1)));
}
}
}

#[test]
fn contents_answer_is_kept_when_no_page_is_listed() {
let mut page = response(Some(Role::Contents));
page.results[0].snippet = Some("y".repeat(300));
page.answer = Some("y".repeat(300));
let narrow = RenderOptions {
contents_chars: 100,
..RenderOptions::default()
};
for out in [
render_text_with(&page, "u", 0, &narrow),
render_markdown_with(&page, "u", 0, &narrow),
] {
assert_eq!(out.matches("y".repeat(100).as_str()).count(), 1);
assert!(!out.contains(&"y".repeat(101)));
}
}

#[test]
fn contents_answer_copy_of_the_page_is_left_out_under_a_budget_only() {
let mut page = response(Some(Role::Contents));
page.provider = "tinyfish".into();
page.results[0].snippet = Some("y".repeat(4000));
page.answer = Some("y".repeat(4000));
let narrow = RenderOptions {
contents_chars: 200,
..RenderOptions::default()
};
for out in [
render_text_with(&page, "u", 5, &narrow),
render_markdown_with(&page, "u", 5, &narrow),
] {
assert!(!out.contains(&"y".repeat(201)));
assert_eq!(out.matches("y".repeat(200).as_str()).count(), 1);
}
// Default output is unchanged.
assert!(render_text(&page, "u", 5).contains(&"y".repeat(4000)));
// Answer-role responses are never trimmed.
let mut answer = answer_fixture();
answer.answer = Some("a".repeat(4000));
let out = render_text_with(&answer, "q", 5, &RenderOptions::compact());
assert!(out.contains(&"a".repeat(4000)));
}

#[test]
fn the_default_contents_cap_matches_todays_output() {
let mut page = response(Some(Role::Contents));
Expand Down
15 changes: 15 additions & 0 deletions crates/tinysearch/src/provider/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,21 @@ fn contents_snippets_keep_the_page_cap_but_search_snippets_stay_bounded() {
);
}

#[test]
fn non_exa_contents_snippets_keep_the_search_snippet_bound() {
let value = json!({"results":[{"url":"https://a","text":"z".repeat(3000)}]});
for (provider, tool) in [("keenable", "keenable_fetch"), ("tavily", "tavily_extract")] {
let page = normalize(provider, tool, &value);
assert_eq!(
page.results[0]
.snippet
.as_deref()
.map(|s| s.chars().count()),
Some(1200)
);
}
}

#[tokio::test]
async fn brave_direct_operations_map_and_normalize() -> TestResult<()> {
for (tool, path, payload) in [
Expand Down
15 changes: 9 additions & 6 deletions crates/tinysearch/src/provider/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,15 @@ fn add_grounding_citations(citations: &mut Vec<Citation>, value: &Value) {
pub(super) fn normalize(provider: &str, tool: &str, value: &Value) -> ExecuteToolResponse {
let mut results = Vec::new();
let mut citations = Vec::new();
// A page-contents tool keeps up to a page's worth of text per result.
let snippet_chars = if role_provider_tool(Role::Contents, provider) == Some(tool) {
MAX_PAGE_CHARS
} else {
MAX_SNIPPET_CHARS
};
// Only Exa honours a per-page bound upstream (`contents_max_chars`), so
// only its contents tool keeps a page's worth of text per result; every
// other tool keeps today's snippet bound.
let snippet_chars =
if provider == "exa" && role_provider_tool(Role::Contents, provider) == Some(tool) {
MAX_PAGE_CHARS
} else {
MAX_SNIPPET_CHARS
};
if let Some(items) = value.get("results").and_then(Value::as_array) {
for item in items {
add_result(&mut results, item, snippet_chars);
Expand Down
Loading