Fix read_cell_output incorrectly reporting all outputs as too large#318148
Open
oded-ist wants to merge 2 commits into
Open
Fix read_cell_output incorrectly reporting all outputs as too large#318148oded-ist wants to merge 2 commits into
oded-ist wants to merge 2 commits into
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adjusts notebook cell output sizing logic to better align with token-budget-based limits and changes which tools are eligible for parallel execution in tool-calling.
Changes:
- Updates large-output detection to compute limits from the token budget (instead of passing byteLength into
getCharLimit). - Removes
ToolName.ReadCellOutputfrom the set of tools eligible to run in parallel.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extensions/copilot/src/extension/tools/node/runNotebookCellTool.tsx | Reworks the “output too large” checks to compute a limit from token budget and compare against output size. |
| extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx | Updates tool parallelization configuration by removing ReadCellOutput from the parallel set. |
Comments suppressed due to low confidence (1)
extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx:1
- Removing
ToolName.ReadCellOutputfromtoolsCalledInParallelcan increase latency when multiple tools are invoked together (since it will no longer be parallelized). If there’s a correctness/sequencing requirement that forces it to run serially, it would help maintainability to add an inline comment explaining the dependency; otherwise consider keeping it in the parallel set.
/*---------------------------------------------------------------------------------------------
DonJayamanne
approved these changes
May 25, 2026
roblourens
approved these changes
May 25, 2026
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
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.