Fix read_cell_output incorrectly reporting all outputs as too large#5093
Open
oded-ist wants to merge 1 commit intomicrosoft:mainfrom
Open
Fix read_cell_output incorrectly reporting all outputs as too large#5093oded-ist wants to merge 1 commit intomicrosoft:mainfrom
oded-ist wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Two related issues caused even tiny notebook cell outputs (e.g. 5 bytes)
to be replaced with "Output is too large to be used as context".
1. Inverted size check in `RunNotebookCellOutput`: `getCharLimit` converts
tokens to chars (×4), but it was applied to the byteLength side and
compared against a token count. Compare byteLength against
getCharLimit(tokenBudget / sizeLimitRatio) instead — the threshold was
16× too small.
2. Remove `ReadCellOutput` from `toolsCalledInParallel`. Tools in that set
are invoked eagerly with a sentinel `{ tokenBudget: 1 }` sizing on the
premise that they don't consume sizing info. `RunNotebookCellOutput`
does — it gates output on `sizing.tokenBudget` — so the sentinel made
every non-empty output trip the size check. Letting it use the normal
lazy path gives it the endpoint's real prompt budget.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related issues caused even tiny notebook cell outputs (e.g. 5 bytes) to be replaced with "Output is too large to be used as context".
Inverted size check in
RunNotebookCellOutput:getCharLimitconverts tokens to chars (×4), but it was applied to the byteLength side and compared against a token count. Compare byteLength against getCharLimit(tokenBudget / sizeLimitRatio) instead — the threshold was 16× too small.Remove
ReadCellOutputfromtoolsCalledInParallel. Tools in that set are invoked eagerly with a sentinel{ tokenBudget: 1 }sizing on the premise that they don't consume sizing info.RunNotebookCellOutputdoes — it gates output onsizing.tokenBudget— so the sentinel made every non-empty output trip the size check. Letting it use the normal lazy path gives it the endpoint's real prompt budget.