Skip to content

fix(display): handle small max_len values in build_tool_preview() - #9477

Closed
kagura-chen wants to merge 1 commit into
NousResearch:mainfrom
kagura-chen:fix/build-tool-preview-max-len
Closed

kagura-chen wants to merge 1 commit into
NousResearch:mainfrom
kagura-chen:fix/build-tool-preview-max-len

Conversation

@kagura-chen

Copy link
Copy Markdown

Problem

build_tool_preview() in agent/display.py breaks its max_len contract when max_len < 4. The truncation logic:

preview[:max_len - 3] + "..."

produces a negative slice index for max_len values of 1, 2, or 3, resulting in output that exceeds the specified max_len.

For example, with max_len=1 and preview="hello":

  • preview[1-3] → preview[-2] → "lo" + "..." → "lo..." (length 5, violates max_len=1)

Fix

When max_len < 4, hard-truncate to max_len without ellipsis, since there is no room for even one character plus "...". For max_len >= 4, the existing ellipsis logic works correctly.

Also fixes an incorrect existing test assertion that allowed len(result) <= 43 instead of <= 40 for max_len=40.

Tests

Added 7 new test cases covering:

  • max_len=0 (unlimited — no truncation)
  • max_len=1, 2, 3 (small values — hard truncate, no ellipsis)
  • max_len=4 (first value where ellipsis fits)
  • Normal truncation with ellipsis
  • No truncation when preview fits within max_len

All 31 display tests pass. Full test suite shows no regressions.

Fixes #9439

When max_len < 4, the truncation logic `preview[:max_len - 3] + '...'`
produces a negative slice index, resulting in output that exceeds max_len.

For max_len < 4, hard-truncate to max_len without ellipsis since there
is no room for even one character plus '...'.

Also fixes the existing test assertion that incorrectly allowed
len(result) <= 43 instead of <= 40 for max_len=40.

Fixes NousResearch#9439
@kagura-chen
kagura-chen force-pushed the fix/build-tool-preview-max-len branch from 1cffe16 to ec39c05 Compare April 15, 2026 00:07
@kagura-chen

Copy link
Copy Markdown
Author

CI note: The test job failure is an upstream issue — main branch tests are also failing (see latest main runs). This PR's changes are unrelated to the test failures.

@kagura-chen

Copy link
Copy Markdown
Author

Closing to reduce PR backlog. The fix is still valid — happy to reopen or resubmit if there's interest. Thanks!

teknium1 pushed a commit that referenced this pull request Sep 12, 2026
`build_tool_preview()`'s generic-key fallback and the cute-message helpers
still truncated with a bare `text[:max_len - 3] + "..."`; for max_len 1-3 the
slice goes negative and returns almost the whole string (27 chars for
max_len=1). `_truncate_preview` already had the guard, so the two code paths
disagreed.

One truncation helper (`_tail_trunc`) with the guard, used everywhere; the
head-truncating `_cute_path` gets the same clamp.

Salvage of PR #48483 by @HeLLGURD (current-code fix); the earliest reports and
patches were #9464 (@LarHope), #9477 (@kagura-agent) and #9497.

Co-authored-by: LarHope <12761142+LarHope@users.noreply.github.com>
Fixes #9439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build_tool_preview() breaks max_len contract for values under 4

1 participant