Skip to content

fix(api): stop JSON.stringify-ing messages before estimateTokens (restores #8368 image estimate) [recovered from #8599] - #8740

Merged
diegosouzapw merged 1 commit into
release/v3.8.49from
recover-8594-estimatetokens
Jul 27, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.49from
recover-8594-estimatetokens

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Recovered from PR #8599 (auto-closed due to a rebase-push race on the contributor's fork — see PR #8599 timeline).

This PR carries the rebased version of the original commit from @Prudhvivuda:

Refs #8594, supersedes #8599 (same fix, same root cause, same regression test). The diff is identical to the rebased commit on the contributor's branch — only the PR/branch location changed because the original contributor fork lost its commit during a force-push collision.

Files (3):

  • open-sse/services/combo.ts (line 1822: estimateTokens(JSON.stringify(attemptBody)) → estimateTokens(attemptBody))
  • open-sse/services/contextManager.ts (5 sites: compressContext initial + layers 1/2/3 + purifyHistory binary-search)
  • tests/unit/8594-compress-image-token-stringify.test.ts (94-line TDD regression — proves within-limit image request NOT compressed, multi-turn image history preserved when fitting, control: oversized text still compressed)

Why this exists as a new PR instead of reopening #8599: the contributor branch tip was overwritten to ed7db3ee (release tip, no PR commits) during the rebase-push race, GitHub auto-closed #8599 and revoked maintainerCanModify. The original author can still re-push their fork to supersede this branch if they prefer to take it back.

…tores #8368 image estimate)

Six production call sites pre-JSON.stringify the messages/body before passing
them to estimateTokens(), forcing the char/4 text path and undoing the #8368
inline base64 image bounded-estimate. A ~500KB image is measured as ~125k tokens
instead of ~1.2k, so compressContext over-estimates and needlessly compresses,
and purifyHistory's binary search prunes image-bearing turns that actually fit.

Pass the structured object directly at all six sites — estimateTokens already
handles both the string and object overloads; the object path walks the
structure for inline base64 image blocks and substitutes the bounded per-image
estimate before measuring the remainder as text.

Sites fixed:
- open-sse/services/contextManager.ts: compressContext initial estimate + after
  Layers 1/2/3, and the purifyHistory binary-search candidate check.
- open-sse/services/combo.ts: fallback compression threshold check.

Adds tests/unit/8594-compress-image-token-stringify.test.ts driving compressContext
directly: a within-limit inline-image request must not be compressed, image-bearing
turns must be retained by purifyHistory, and oversized text is still compressed
(control). Fails before the fix, passes after.

Closes #8594

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Owner Author

Validated in local merge-train /tmp/train1d-20260727-090022-suite.log on .113 @ 029cdf4215cf465f0e1716ac9f84a84692b1e881 (full unit suite green on CI-equivalent host)

@diegosouzapw
diegosouzapw merged commit f9899c5 into release/v3.8.49 Jul 27, 2026
20 checks passed
@diegosouzapw
diegosouzapw deleted the recover-8594-estimatetokens branch July 28, 2026 06:47
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.

2 participants