Studio: size a large tool result to the window the model is running with - #9384
Conversation
The cap on fetched page text was a flat 16,000 characters, roughly 4,000 tokens. That is nothing on a 128k model and larger than the whole prompt budget on a small one. Measured on a 4,864-token model. A two-message thread: an 11-token question and one assistant turn that searched, was told by the snippet result to fetch the page for full content, and did. The page came back at 12,295 characters, roughly 3,073 tokens, against a 3,648-token prompt budget. The request was refused at 8,995 tokens with latest_turn_role: tool and dropped_messages: 0. Nothing downstream could save it. The fit protects the newest turn, so compaction may not drop the oversized result, and there was nothing else in the thread to drop; the whole conversation was the tool call. The user was advised to shorten a conversation consisting of one question. The cap now falls back to a third of the loaded window when that is smaller, with a floor so a fetch never returns a fragment too clipped to answer from. Above roughly an 11k window it returns the old constant unchanged, so no model that could already afford a page is affected, and an unreadable window stays 'unknown' and keeps the constant rather than silently shrinking every fetch. The parameter default moves to None and resolves per call: bound at import it would freeze the constant before any model is loaded, which is precisely when the window is not yet known.
for more information, see https://pre-commit.ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5e4104eab
ℹ️ 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".
| return ctx | ||
| except Exception: # noqa: BLE001 -- an unreadable window is "unknown", never an error | ||
| return None | ||
| return None |
There was a problem hiding this comment.
When the active chat model is a native/Transformers model rather than GGUF, get_llama_cpp_backend().is_loaded is false and this unconditional return reports an unknown window. _page_char_budget() consequently retains the 16,000-character cap, so small native models still encounter the oversized tool-result failure this change is intended to prevent. The existing research_runs._loaded_context_length and routes.inference._monitor_context_length implementations both consult the inference orchestrator after the llama.cpp probe; this reader needs the same fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, and my own docstring claimed otherwise. It said "Mirrors research_runs._loaded_context_length" while stopping at the llama.cpp probe, so on a native/Transformers chat is_loaded is false, the reader reported unknown, and _page_char_budget kept the full 16,000-character cap. That is exactly the overflow this change exists to prevent, reintroduced on the models least able to absorb it.
Added the orchestrator branch the precedent uses: _peek_inference_backend, then models[active_model_name]["context_length"], backend.context_length, backend.max_seq_length in that order. Peek rather than construct, since constructing reaches get_default_models() -> get_device() and would block the loop on the torch import just to answer "nothing is loaded". The llama.cpp branch now falls through instead of returning None, so an unreadable GGUF probe no longer masks a readable native one. Every failure still means unknown, never an error.
| llama = get_llama_cpp_backend() | ||
| if getattr(llama, "is_loaded", False): | ||
| ctx = getattr(llama, "context_length", None) | ||
| if isinstance(ctx, int) and ctx > 0: | ||
| return ctx |
There was a problem hiding this comment.
When an external-provider request runs Studio's local tool loop while a GGUF model remains resident, this probe uses the resident model's context even though routes/inference.py explicitly routes that request without touching the local GGUF. A small resident model therefore silently truncates pages for a large cloud model, while a large resident model leaves the full 16,000-character result for a small OpenAI-compatible endpoint and can reproduce the overflow. External requests need request-specific context information or must be treated as unknown rather than inheriting unrelated process-global state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. studio_tool_loop.py even says so in a neighbouring comment ("Studio cannot measure an external model's window") while the page budget was reading the resident GGUF regardless, so both directions were live: a small resident model truncating pages for a large cloud model, and a large resident model handing the full 16,000 characters to a small OpenAI-compatible endpoint.
Fixed by making the budget read a request-scoped value rather than process-global state. execute_tool takes context_tokens and sets a ContextVar for the call; _page_char_budget uses it when present. The sentinel matters: an explicit 0 means "asked, and unknowable" and keeps the default cap, whereas an absent value keeps the probe. Only that distinction stops the inheritance coming straight back.
studio_tool_loop (the external-provider path) passes 0. The local loops pass nothing and keep the probe, which is the correct reading for them, so the diff stays small. The ContextVar is set unconditionally at the top of every execute_tool call, so a value from an earlier call on the same thread can never be read by a later one and no try/finally reset is needed.
Verified: unset -> 16000, external -> 16000, a 4864-token window -> 6809.
Two ways the cap read the wrong window, both raised in review.
The reader stopped at the llama.cpp probe while its own docstring claimed to
mirror research_runs._loaded_context_length. On a native/Transformers chat
is_loaded is false, so it reported "unknown", _page_char_budget kept the full
16,000-character cap, and small native models hit exactly the oversized
tool-result failure this change exists to prevent. Added the orchestrator
branch the precedent uses: _peek_inference_backend, then
models[active_model_name]["context_length"], backend.context_length,
backend.max_seq_length. Peek rather than construct, since constructing reaches
get_default_models() -> get_device() and would block the loop on the torch
import just to answer "nothing is loaded". The llama.cpp branch now falls
through instead of returning None, so an unreadable GGUF probe no longer masks
a readable native one.
The budget also read process-global state. An external-provider request runs
Studio's tool loop without touching a resident GGUF, so it inherited that
GGUF's window in both directions: a small resident model truncating pages for a
large cloud model, and a large resident model handing the full 16,000
characters to a small OpenAI-compatible endpoint. studio_tool_loop already says
as much in a neighbouring comment ("Studio cannot measure an external model's
window") while the cap ignored it.
execute_tool now takes context_tokens and sets a ContextVar for the call, which
_page_char_budget prefers over the probe. The sentinel carries the distinction
that matters: an explicit 0 means "asked, and unknowable" and keeps the default
cap, while an absent value keeps the probe. studio_tool_loop passes 0; the
local loops pass nothing and keep the probe, which is correct for them, so the
diff stays small. The ContextVar is set unconditionally at the top of every
execute_tool call, so a value from an earlier call on the same thread can never
be read by a later one and no try/finally reset is needed.
Verified: unset -> 16000, external -> 16000, a 4864-token window -> 6809.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
for more information, see https://pre-commit.ci
Same defect as the page cap, on a different constant. Tool results were capped at a fixed 16,000 characters, roughly 4,000 tokens, with no relation to the loaded context. Observed live on a 5,120-token window: two requests refused at 7,043 and 6,684 tokens, both on the code tools. Worse than merely large. The result lands in the NEWEST turn, and the fit protects that turn, so compaction cannot drop the one thing that does not fit and the request is irreducible rather than trimmable. The user is then told to shorten a conversation whose bulk is a single tool result. _result_char_budget generalises the page helper and both now share it, so a fetched page and a terminal result are lowered by the same rule and neither is touched above roughly an 11k window. _truncate resolves its limit per call rather than binding the constant at import, which would freeze it before any model is loaded, exactly when the window is still unknown. At 5,120 tokens the cap drops from 16,000 to 7,168 characters, which is what brings the live 7,043-token request back under the window. Also adds a reset to the autouse fixture: execute_tool sets the request-scoped window deliberately, and without a restore one test decided another's budget.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 636fa8bb2a
ℹ️ 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".
| ctx = _loaded_context_tokens() if scoped is _UNSET_CONTEXT_TOKENS else scoped | ||
| if not ctx: | ||
| return _MAX_PAGE_CHARS | ||
| return max(_MIN_PAGE_CHARS, min(_MAX_PAGE_CHARS, int(ctx * 4 * _PAGE_CONTEXT_SHARE))) |
There was a problem hiding this comment.
When the fetched page contains CJK, emoji, or similarly token-dense text, multiplying the context window by four does not reserve 35% of the window; it can admit text approaching 140% of the entire window. For example, a 4,864-token model receives a 6,809-character page budget, while the repository's own estimate_messages_tokens_dense documents that CJK runs near one token per character, so the tool result alone can exceed the model's total context before system, user, and reply tokens are included. This reproduces the irreducible overflow the change is intended to prevent; use a conservative dense-text calculation or an available tokenizer rather than a fixed four-characters-per-token conversion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed and fixed in 7056c57. I measured it rather than reasoning about it, and the conclusion holds even though the premise is a little off.
Reproduced on a real page. Fetched zh.wikipedia.org/wiki/人工智能, ran it through the repo's own html_to_markdown(main_content = True), took the PR's budget int(ctx * 4 * _PAGE_CONTEXT_SHARE) = 6809 chars on a 4864-token window, and counted:
cl100k_base 4070 tokens 84% of the window
o200k_base 3759 tokens 77% of the window
So a budget that says 35% is 77 to 84%, in the newest turn, which the fit protects from eviction. That is exactly the irreducible refusal this PR exists to prevent, and fetching a Chinese Wikipedia page is ordinary input, not an exotic case.
On the premise: near one token per character is right for cl100k, but stale for the tokenizers this actually runs on. Measured on the extracted prose, Qwen3 gives 1.60 and 1.40 chars/token for zh and ja, Llama 3.2 gives 1.33 and 1.42, o200k 1.37 and 1.31. So your 140% of the entire window is overstated; the real figure is 77 to 95% of the prompt budget. The gap to 4.0 is still 2.5x to 4.5x, which is all the finding needs.
One thing worth adding: literal CJK is not the only dense input on such a page. The percent-escaped link targets (/wiki/%E7%9F%A5%E8%AF%86) measured 1.29 to 1.5 chars/token, the same non-ASCII bytes spelled in ASCII. The zh page is only 9.8% literal non-ASCII yet still runs 1.75 chars/token overall because of them, which is also why estimate_messages_tokens_dense, charging only literal non-ASCII, undercounts it at 2165.
The fix takes a tokenizer-free route: one linear pass charging non-ASCII characters and percent-escapes a token each, applied as a ceiling on the existing char caps, for pages and for terminal and python results alike. Same page after: 2323 chars, 1473 cl100k tokens, 30%. English is unchanged to within the truncation notice (6809 -> 6822 chars, 1702 -> 1707 tokens). An unknown window is a no-op, the readable floor still holds, and a caller-pinned smaller cap still wins.
Seven new cases in tests/test_web_page_cap_fits_window.py; no existing assertion was relaxed. 1304 passed across the page, extraction, binary-guard, access-policy, external-tool-budget and context-overflow suites.
danielhanchen
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7056c577c6
ℹ️ 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".
| else: | ||
| spent += 1.0 if ord(text[index]) > 127 else 0.25 |
There was a problem hiding this comment.
On small-window models, terminal or Python output containing hashes, minified JSON, base64, source code, or other token-dense ASCII is still charged at four characters per token, although the llama.cpp admission path elsewhere in this repository explicitly notes that such output commonly costs one token per two or three characters. A 5,120-token request can therefore retain 7,168 ASCII characters that consume roughly 2,400–3,600 tokens rather than the promised 1,792-token share, recreating the irreducible overflow this change is intended to prevent. Fresh evidence beyond the earlier dense-text finding is that the revised correction now explicitly assigns every ordinary ASCII character a fixed 0.25 cost rather than measuring or conservatively estimating these common tool outputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. This is the concrete colliding pair I said I would need, so the standing rejection of the ASCII-drift argument does not cover it.
The difference from the earlier version is that this one produces an actual refusal, not a ratio. _dense_prefix_chars charges every ASCII character a flat 0.25, so dense ASCII passes the cap untouched. On a 5,120-token window the cap admits 7,168 chars against a promised 1,792-token share, and I measured what that costs with Qwen3-4B, Llama-3.2 and tiktoken:
base64 payload.bin 7168 chars -> 5361 Qwen3 / 5205 cl100k 105% of the WHOLE window
hexdump -C 7168 chars -> 5540 Qwen3 108%
sha256sum * 7168 chars -> 5109 Qwen3 100%
English prose 7168 chars -> 1230 Qwen3 24%
End to end through the real fit_rolling_context and the real Qwen3 template, a four-message thread with an 8-token question and one base64 result: 5475 tokens against a 3,840-token prompt budget, fits False, dropped_messages 0. Refused irreducible. That is this PR's own failure mode, after this PR, on an ordinary terminal command.
What decided the fix was that no character rule works. I measured a word-structure heuristic (plain-word vs mixed-class, 4 vs 2.5 chars/token) and it still undercharges base64 about 2x while overcharging English 39%, which would shrink every page this PR promises to leave alone. A 76-character base64 line really costs 57 Qwen3 tokens.
So when a tokenizer is already serving the request, ask it. _exact_prefix_chars uses the same count_chat_tokens that the RAG admission path and five other sites in llama_cpp.py already use for exact fitting. Never grows a budget, at most 3 passes, skipped when no local model is loaded or the counter raises, gated on the backend's window matching the one the budget was sized against so a resident GGUF cannot price a native or external request, and not run below the floor. English 1 call / 2.3 ms, base64 2 calls / 13.8 ms.
After: the same session comes back 2552 chars, 1824 tokens against a 1792 share, admitted.
This PR is already merged, so the fix is in #9421 rather than here.
| ctx = _loaded_context_tokens() if scoped is _UNSET_CONTEXT_TOKENS else scoped | ||
| if not ctx: | ||
| return cap | ||
| return max(_MIN_PAGE_CHARS, min(cap, int(ctx * 4 * _PAGE_CONTEXT_SHARE))) |
There was a problem hiding this comment.
When UNSLOTH_TOOL_RESULT_MAX_CHARS is configured below 2,000 and a local model window is known, this expression returns at least _MIN_PAGE_CHARS, so _tool_result_char_budget() silently raises the administrator's model-visible output cap. For example, a configured 500-character cap becomes 2,000 characters, unlike the hosted-result path that still honors 500; the readability floor should not exceed the cap argument.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and it is a regression this PR introduced. Reproduced:
_MAX_OUTPUT_CHARS = 500
window UNKNOWN : _tool_result_char_budget()=500 hosted=500
window 8192 : _tool_result_char_budget()=2000 hosted=500
window 1024 : _tool_result_char_budget()=2000 hosted=500
The floor sits outside the cap:
return max(_MIN_PAGE_CHARS, min(cap, int(ctx * 4 * _PAGE_CONTEXT_SHARE)))
_MAX_OUTPUT_CHARS is _env_int("UNSLOTH_TOOL_RESULT_MAX_CHARS", 16000) and _env_int accepts anything above 0, so 500 is a supported configuration. Before this PR _truncate(text, limit = _MAX_OUTPUT_CHARS) honoured it exactly. Now the one function whose job is to LOWER the cap raises it fourfold, and worst on the smallest windows, which are the ones this PR is for.
Your divergence point is right too: studio_tool_loop._truncate_for_model reads tools_module._MAX_OUTPUT_CHARS directly and still cuts at 500, so the same install gets 500 characters from a hosted result and 2,000 from a local one, when that function's docstring exists to promise they match.
Fix is the clamp order, min(cap, max(_MIN_PAGE_CHARS, ...)). The floor is for when the WINDOW is what makes a result small, not a licence to exceed what the install configured. _page_char_budget has the same shape but its cap is the constant _MAX_PAGE_CHARS = 16000, which cannot be configured below 2,000, so it is left alone.
I checked whether an existing test asserted the wrong thing here and none did: test_an_explicit_limit_is_still_a_ceiling_not_a_floor covers _dense_char_limit's argument, which was already clamped correctly, and test_an_explicit_limit_still_wins covers _truncate's argument. Both bypass _result_char_budget, so nothing had to be relaxed. Four new cases cover the configured-cap path.
Merged already, so this is in #9421.
1 participant
What happened
A two-message thread on a 4,864-token model:
web_searchtwiceBoth searches succeeded. The first returned snippets and ended with the tool's own advice:
The model did exactly that, and the fetched page came back at 12,295 characters, roughly 3,073 tokens, against a 3,648-token prompt budget. The request was then refused:
Nothing downstream could recover from it. The fit protects the newest turn, so compaction may not drop the very result that does not fit, and there was nothing else to drop: the whole conversation was the tool call. What the user saw was
Message too long: 8995 tokens exceeds the 4864-token context window. Try increasing the Context Length in Model settings, or shorten the conversationon a thread containing one 11-token question.The same shape then showed up without any page fetch at all: 7,043- and 6,684-token requests refused on a 5,120-token window, because a 16,000-character terminal result landed in the protected newest turn. The cap is the same constant, so the fix has to be the same fix.
The cause
A flat constant, independent of the model. 16,000 characters is about 4,000 tokens: negligible on a 128k model, and 110% of the entire window here. The cap never fired for the page, because 12,295 < 16,000, so a result that could not possibly fit was passed through untouched.
The change
Every large tool result is sized to the window the model is actually running with rather than to a fixed constant. Fetched pages, terminal output and python output all resolve to
min(constant, 35% of the window)with a floor (_MIN_PAGE_CHARS) so a result never comes back too clipped to answer from. A third-ish share because the same window also has to hold the system prompt, the carried-forward block, the user's turn, the call itself and room to reply.The window is read per request: a
ContextVarset byexecute_toolwins, and otherwise a probe tries llama.cpp first and then the orchestrator, so native and Transformers chats are covered too. The external-provider tool loop passescontext_tokens=0, meaning "asked, and unknowable", so a cloud request neither inherits nor is throttled by whatever GGUF happens to be resident.A character cap only reserves its share for English. Four characters per token is an English rate; measured with Qwen3, Llama 3.2 and tiktoken against real fetched pages, CJK prose runs 1.3 to 1.6 characters per token, and so do the percent-escaped links a CJK page is full of (
%E7%9F%A5%E8%AF%86), which are the same non-ASCII bytes spelled in ASCII. On a realzh.wikipedia.orgarticle the "35%" budget measured 4,070 tokens against a 4,864-token window, 84% of it - the same irreducible refusal, reached by a page that is entirely ordinary to fetch. So the char budget is now also capped by a single linear pass charging non-ASCII characters and percent-escapes a token each, the ruleestimate_messages_tokens_densealready documents. Same page after: 1,473 tokens, 30%. English is unchanged to within the truncation notice.Deliberately narrow:
max_charsstill wins, so callers sizing their own budget and the extraction tests that pin exact output are unaffected.The parameter default moves from
_MAX_PAGE_CHARStoNoneand resolves per call. Bound at import it would freeze the constant before any model is loaded, which is exactly when the window is not yet known.Tests
tests/test_web_page_cap_fits_window.py: the 4,864-token window no longer admits the page that caused the refusal; 16k/32k/128k windows are unchanged; an unknown window keeps the constant; a tiny window still returns a readable result; the budget never exceeds the absolute cap; an unreadable backend reads as unknown rather than raising (exercising the real reader, not a stub of it); an explicit size still wins; the terminal and python caps follow the window; and for the dense case, a CJK page is cut to the share it was promised, an English page is left exactly as it was, percent-escapes are charged like the bytes they encode, the readable floor still holds, an unknown window leaves a dense page alone, and a dense terminal result is sized too.test_web_page_cap_fits_window,test_web_fetch_extraction,test_web_access_policy,test_web_fetch_binary_guard,test_external_tool_truncated_and_budget,test_context_overflow_truncation: 1304 passed.Not fixed here
The same session hit
Research could not be completed: Local model report reached its output limit before completionon a Deep Research run asking for a 356-day itinerary. That message is accurate rather than a bug:_completion_hit_context_wallwas false, so the report genuinely exhausted its output budget rather than the window. A 356-day plan does not fit in one response at any small context. Worth a separate look is that the sibling error string for the context-wall case names a remedy ("Increase Context Length in chat settings or reduce the research evidence size") while this one names none.