Skip to content

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

Closed
Prudhvivuda wants to merge 0 commit into
diegosouzapw:release/v3.8.49from
Prudhvivuda:fix/8594-estimatetokens-image-bypass
Closed

Prudhvivuda wants to merge 0 commit into
diegosouzapw:release/v3.8.49from
Prudhvivuda:fix/8594-estimatetokens-image-bypass

Conversation

@Prudhvivuda

Copy link
Copy Markdown
Contributor

Summary

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 for those paths. estimateTokens(string) takes Math.ceil(str.length / CHARS_PER_TOKEN); the image-detection walk (extractImageTokens) only runs for the object overload introduced by #8368. Stringifying first strips the structural type information the fix depends on.

Impact — for any Chat Completions request carrying inline base64 images ({ type: 'image_url', image_url: { url: 'data:image/...' } }) that triggers compression:

  1. compressContext over-estimates (a ~500 KB image → ≈125 000 tokens via base64-as-text instead of the correct ~1 200-token estimate) → thinks the context is way over the limit and enters aggressive compression.
  2. purifyHistory binary-search prunes image-bearing turns unnecessarily → silent context loss.
  3. combo.ts fallback compression fires for requests that fit comfortably.

Fix

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, substitutes the bounded per-image estimate, then measures 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 (5 sites).
  • open-sse/services/combo.ts — fallback compression threshold check (1 site).

Test plan

New TDD guard tests/unit/8594-compress-image-token-stringify.test.ts drives compressContext directly (fails before the fix, passes after):

  • a within-limit inline-image request must not be compressed;
  • purifyHistory retains all image-bearing turns when they fit;
  • an oversized text request is still compressed (control — no regression).

Validated on Node v22.8.0 (supported range):

  • New suite: 3/3 pass (2 of 3 fail on base 4053e2314, confirming reproduction).
  • #8368 image-token suite: 8/8 pass (no regression).
  • Compression/context suites (context-manager, compression-pipeline-inflation-guard, compression-noop-guard, combo-context-length, context-window-reconcile, compression-tokens): 54/54 pass.
  • npm run typecheck:core clean; eslint clean on all changed files.

Closes #8594
Refs #8368, #8560

@mergify

mergify Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ The sha of the head commit of this PR conflicts with #7076. Mergify cannot evaluate rules on this PR. Once #7076 is merged or closed, Mergify will resume processing this PR. ⚠️

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.

fix(api): estimateTokens callers JSON.stringify before calling — bypasses #8368 image-token fix in compressContext and purifyHistory

2 participants