Skip to content

fix(runtime): keep every placeholder when bounding a multi-result tool message - #4456

Merged
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/compaction-multi-result-tool-message
Sep 2, 2026
Merged

fix(runtime): keep every placeholder when bounding a multi-result tool message#4456
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/compaction-multi-result-tool-message

Conversation

@liuxiaocs7

Copy link
Copy Markdown
Member

Summary

fitHistoryCompactMessages bounds a compaction request by replacing old, oversized tool-result payloads with a placeholder. Each replacement copied from the original message.content, so a single tool message that batches several results (one per parallel tool call) kept only the last placeholder while every earlier payload stayed at full size — even though the budget accounting (estimatedChars) had already credited their removal. The helper could therefore return a history that still exceeds maxInputEstimatedTokens yet treat it as "fit" (or, at other sizes, throw input_too_large for input that would actually fit), failing at exactly the case it exists to handle.

The fix accumulates replacements across a message's parts, matching the pattern already used in active-tool-result-prune.ts.

Fixes #4454

Verification

  • npm --workspace @maka/runtime run typecheck — clean
  • node --test dist/__tests__/history-compact-input-fit.test.js — the new bounds every oversized result in a multi-result tool message case fails on main (returns 16974 chars for a 1000-char budget, one full payload surviving) and passes with this change; the three existing cases still pass
  • npx biome check on both changed files — clean

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — diagnosis, the fix, and the test, under human review of record. A Generated-by: Claude Code trailer is on the commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…l message

fitHistoryCompactMessages copied each replaced tool-result from the
original message content, so a tool message batching several results
(one per parallel tool call) kept only the last placeholder while the
budget accounting credited all of them. The returned history could then
still exceed the requested budget yet be treated as "fit". Accumulate
replacements across a message's parts, matching active-tool-result-prune.

Fixes apache#4454

Generated-by: Claude Code
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 1, 2026

@Astro-Han Astro-Han 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.

Confirmed the bug and the fix.

const content = [...message.content] copied from the original content every iteration, so with two oversized results the second replacement overwrote the first — while estimatedChars had already been credited twice. The early return bounded then hands back a history that claims to fit and doesn't. Parallel tool calls make that the common shape, not the corner case, which is the part that makes this worth fixing rather than noting.

The accumulating content is right, including at the early return. Reads clean to me.

@Astro-Han
Astro-Han merged commit 619cd52 into apache:main Sep 2, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(runtime): history compaction keeps earlier oversized tool results in a multi-result tool message

2 participants