Repository navigation
fix(core): keep the tail when tool output is truncated - #62719
vvvzhangshuo wants to merge 1 commit into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The main case works. I ran ToolOutput.truncate on the base and with this PR: 100 lines with max_lines: 10 now gives lines 1-5, the marker, then lines 96-100, where the base gave lines 1-10. The tool-output tests and typecheck:tsc pass. One mcp.test.ts failure happens on the base too, so it isn't from this change.
Public API: no API, config or schema change. The only visible change is the model-facing marker text: showing lines A-B and C-D of N, and showing 0 lines when nothing fits.
Two things are worth fixing before merge (details inline):
- Lines move around attachments. When a text item is split, its lines can move to the other side of an attachment. That contradicts the description's claim that attachments keep their position.
- Unused tail budget isn't given back to the head. Output whose last line is too big now shows fewer lines than before, and no tail at all.
A smaller point: because the marker's size, including the full file path, is reserved up front, a small max_bytes (under roughly 150 bytes) now shows no content at all, where the base still showed some lines.
| const headText = head.join("\n") | ||
| const tailText = tail.join("\n") | ||
|
|
||
| // Attachments keep their position: the head and marker replace the first text item, the tail the last one. |
There was a problem hiding this comment.
Attachments stay in place in the array, but the text around them doesn't. All head lines go into the first text item and all tail lines into the last, whichever item they came from. With ["a1..a6", file, "b1"] and max_lines: 4, I get ["a1\na2\n<marker>", file, "a6\nb1"]: a6 came before the image in the original and now comes after it. With ["a1", file, "b1..b5"], b1 ends up in front of the image. For tools that interleave text and images (MCP, read), the model then sees text next to the wrong attachment. Could the selection track which item each kept line came from (head or tail) and emit the lines back into that item, with the marker going into the item where the gap starts?
| headBytes += size | ||
| } | ||
|
|
||
| const tail: string[] = [] |
There was a problem hiding this comment.
The head is capped at half the bytes (headBudget) and half the lines (headLineBudget) before the tail is tried. When the tail can't use its share, nothing goes back to the head. Example: 30 short lines followed by one 2,000-byte line, with max_lines: 10 and max_bytes: 1000. The base shows 10 lines; this PR shows lines 1-5 and no tail. A long final line is common (a JSON blob, a stack trace on one line). Picking the tail first, or extending the head after the tail loop with whatever budget is left, would keep this case at least as good as before.
204f2bb to
f1fcf3f
Compare
|
Addressed the review points in f1fcf3f:
|
There was a problem hiding this comment.
Both earlier points are addressed. Kept lines now go back into the text item they came from, so text no longer moves across attachments. When the tail ends early, the head takes the leftover budget: an oversized last line now shows as many lines as before. Content also no longer disappears when max_bytes is very small. I re-checked these cases by calling ToolOutput.truncate directly, along with the 100-line case from #62718 and items that end in a newline. The tool-output tests and typecheck:tsc pass.
Public API is unchanged; only the model-facing marker text differs.
No new concerns.
|
Gentle ping — both automated review rounds are clean ( I want to be explicit about why I think this matters: in my sessions the decisive information is often at the end of a long tool result — a skill's final checklist, an MCP result field, or the actual error at the bottom of a build log. Dropping that tail silently costs task success, forces extra turns to recover the missing part, and usually adds more context than the bounded tail would have. Keeping both ends is a small, bounded change that avoids all three. Happy to narrow the scope (for example, only MCP/skill results) if generic head+tail is not the direction you want. Thanks! |
Issue for this PR
Fixes #62718
Type of change
What does this PR do?
Generic tool output keeps the first lines and discards the rest. Shell output already keeps the end (
packages/core/src/shell.tspages fromsize - maxBytes), so a cut result behaves differently depending on which tool produced it. This change keeps both ends for generic tool results.max_lines: the tail can use the head's unused line budget, and the total never exceeds the limit.[showing lines 1-5 and 96-100 of 100; full output saved to ...]. It stays compatible with the line counts added in fix(core): show line counts when tool output is truncated #51200 and withread'soffset.max_bytes. Whenmax_bytesis smaller than the marker itself, content keeps the old budget behavior instead of disappearing.metadata.truncatedandoutputPathare unchanged.Example with
max_lines: 10and 100 lines of output:Before: lines 1-10, then
[showing lines 1-10 of 100; ...].After: lines 1-5, the marker, then lines 96-100.
Not in scope: a single line longer than the byte budget is still dropped as before (minified JSON), and line ranges only cover whole lines. I can follow up if that is wanted.
How did you verify your code works?
tool_output.max_lines: 10in the added test; before the changetruncatereturned lines 1-10 only.cd packages/core && bun test test/tool-output.test.ts(16 pass)cd packages/core && bun run typecheckandbun run typecheck:tscbun run lintbun run checkfrom the repository rootChecklist