Skip to content

Studio: reject binary web_search fetches instead of decoding them into replacement chars - #7130

Merged
danielhanchen merged 17 commits into
unslothai:mainfrom
oobabooga:fix/web-fetch-binary-guard
Jul 15, 2026
Merged

danielhanchen merged 17 commits into
unslothai:mainfrom
oobabooga:fix/web-fetch-binary-guard

Conversation

@oobabooga

@oobabooga oobabooga commented Jul 14, 2026 •

Copy link
Copy Markdown
Member

Closes #7084

Problem

When web_search fetched a URL serving binary content, _fetch_page_text decoded the bytes with errors="replace". A PDF could therefore add thousands of U+FFFD replacement characters to the model context, causing the model to emit corrupted output and leaving later turns unusable.

This was reproduced with the reported model (unsloth/Qwen3.6-27B-MTP-GGUF) and the IRS f1040.pdf, which previously produced 6593 replacement characters.

Fix

_fetch_page_text now classifies and sniffs response bodies before returning them to the model:

  • Unambiguous binary MIME types such as PDF, images, and archives are rejected before decoding.
  • Text MIME types, unknown application types, generic application/octet-stream downloads, and ambiguous Office MIME labels continue to byte sniffing, preserving readable logs, configs, SQL, CSV exports, form data, and source artifacts.
  • Common PDF, archive, image, compression, modern Office ZIP, and legacy Office OLE signatures catch mislabeled binary. Optional UTF-8 BOM and whitespace prefixes are ignored during signature detection.
  • Control and undecodable-character density catches mislabeled or unlabeled binary, including valid UTF-8 control bytes.
  • Undeclared or Latin-1-labeled cp1252 text is recovered only when the bytes contain strong ASCII text structure, preventing high-byte binary from being accepted as text.

Rejected bodies return a short (non-text content: ...) or (binary content, ...) placeholder instead of poisoning model context.

Verification

Added focused coverage for MIME classification, unknown application and octet-stream sniffing, ambiguous Office labels, mislabeled and unlabeled binary, prefixed magic signatures, control-byte density, Latin-1 and cp1252 recovery, ANSI text, threshold boundaries, and clean HTML.

Local focused and related verification: 56 passed. Source formatting, Ruff checks, compilation, and whitespace checks also pass.

@oobabooga
oobabooga requested a review from danielhanchen as a code owner July 14, 2026 21:07

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to prevent the web search fetcher from decoding binary bodies (such as PDFs, images, and archives) into text, which avoids flooding the model context with replacement characters (U+FFFD). It implements a content-type classifier and a fallback replacement-character ratio check for mislabeled or unlabeled binary content, accompanied by comprehensive unit tests. The review feedback suggests updating the type annotation of content_type in _is_texty_content_type to str | None to ensure type safety and prevent static analysis warnings.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread studio/backend/core/inference/tools.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e21aed7bc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py
Comment thread studio/backend/core/inference/tools.py Outdated
Comment thread studio/backend/core/inference/tools.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fcbc1a943

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
Comment thread studio/backend/core/inference/tools.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef43072848

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
Comment thread studio/backend/core/inference/tools.py
@oobabooga

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10592853fc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
Comment thread studio/backend/core/inference/tools.py
@oobabooga

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bfd58252a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
Comment thread studio/backend/core/inference/tools.py Outdated
@oobabooga

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 14dca2482e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Resolve import conflict in studio/backend/core/inference/tools.py by keeping
both codecs (web fetch binary guard) and fnmatch (chat tool permission levels).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76de937a95

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4eb12832c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

raw_html = raw_bytes.decode(declared or bom_codec or "utf-8", errors = "replace")

# Catch mislabeled or unlabeled binary, including valid UTF-8 controls.
if _looks_binary(raw_html):

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 Require ASCII evidence for Latin-1 bodies

For responses declared as text/plain; charset=iso-8859-1, high-byte binary payloads such as long runs of bytes in 0xA0..0xFF decode to printable Latin-1 characters, so _looks_binary(raw_html) sees no controls or replacement chars and this branch accepts them without ever applying _has_single_byte_text_evidence. That leaves exactly the “high-byte binary accepted as text” case unguarded when a server supplies a Latin-1 charset; apply the ASCII-structure check before accepting declared Latin-1/cp1252-style decodes, not only when the first decode already looks binary.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed in c7fbec2: the ASCII-structure gate now also applies to declared iso-8859-1/windows-1252 decodes, so high-byte binary mislabeled as Latin-1 is rejected. Scoped to the Latin family so legitimate non-Latin single-byte pages (Cyrillic, Greek) with dense high bytes are not dropped. Verified against a full declared-encoding matrix.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On reflection this is reverted (3b7bebe). The only available signal (ASCII ratio) cannot separate dense-accented Latin-1 text from high-byte binary, so gating declared Latin-1/cp1252 regressed legitimate text (see the follow-up review). The mislabeled-binary-as-Latin-1 case matches pre-PR behavior and is already bounded by the MIME, magic-byte, and control-density guards, so the declared single-byte path is left to decode and keep.

Latin-1 and cp1252 decode every byte to a printable character, so a high-byte
binary body declared as iso-8859-1/windows-1252 decoded cleanly and slipped
past the control-character binary check. Apply the existing ASCII-structure gate
to those declared decodes as well. Scoped to the Latin family so legitimate
non-Latin single-byte pages (Cyrillic, Greek) are not rejected.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7fbec216c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/core/inference/tools.py Outdated
Comment on lines +3893 to +3895
elif declared_codec in ("iso8859-1", "cp1252") and not (
_has_single_byte_text_evidence(raw_bytes)
):

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 dense Latin-1 text

When a server explicitly declares iso-8859-1 or cp1252 and the body is valid text but not ASCII-dominated, such as a text/plain list containing mostly accented names or other Western characters, this branch returns the binary placeholder solely because fewer than 75% of the raw bytes are ASCII. That regresses legitimate declared single-byte text that was previously decoded successfully; the ASCII-structure gate should be limited to unlabeled/fallback recovery or otherwise avoid rejecting cleanly decoded declared text.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch, and agreed. Reverted in 3b7bebe: declared iso-8859-1/windows-1252 bodies are decoded and kept as before, so legitimate dense-accented Western text is no longer dropped.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b7bebe291

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

(codec for bom, codec in _UNICODE_BOM_CODECS if raw_bytes.startswith(bom)),
None,
)
raw_html = raw_bytes.decode(declared or bom_codec or "utf-8", errors = "replace")

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 Reject Latin-1 declared binary before returning it

When a response declares charset=iso-8859-1 or windows-1252, decoding here can map arbitrary high-byte binary to printable characters, so _looks_binary(raw_html) stays false and the blob is returned to the model instead of the new placeholder. For example, a text/plain; charset=iso-8859-1 body with no magic header and mostly bytes 0xA0..0xFF passes this guard and still poisons context with unreadable data; apply the same ASCII-structure sniffing used for the cp1252 rescue to declared Latin-1/cp1252 decodes before accepting them as text.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Declining this: it is the direct inverse of the earlier review that (correctly) flagged the same gate for dropping legitimate dense-accented Latin-1 text. The ASCII-ratio signal cannot separate dense Latin-1 text from high-byte Latin-1 binary (byte-identical), so gating declared Latin-1/cp1252 regresses real text. This path requires a server mislabeling binary as iso-8859-1 text, matches pre-PR behavior (not a regression), and any true binary is already bounded by the MIME, magic-byte, and control-density guards. Leaving declared single-byte responses to decode and keep.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 3b7bebe291

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Integrate the web-fetch binary guard into main's refactored fetch path. Main
split _fetch_page_text into a low-level _fetch_url_raw (returns
(error, text, content_type) with a wall-clock budget) plus a high-level
renderer. The MIME/magic/BOM/control-density guard now runs in _fetch_url_raw
before decoding and returns its placeholder as the error string, which
_fetch_page_text surfaces unchanged. Keep both the guard helpers and main's new
GitHub README helpers. Update the test double to advance its read cursor so
main's chunked _read_capped_body reaches EOF.
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 29e6ffe8a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@danielhanchen
danielhanchen merged commit 770f92e into unslothai:main Jul 15, 2026
16 of 34 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: f8296d3591

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Search Agent doesn't reply with results, must be prompted again

3 participants