From 1cfbfc044a4aad99a6e953265aac6532c82ff2f8 Mon Sep 17 00:00:00 2001 From: MumuTW <42820974+MumuTW@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:52:38 +0800 Subject: [PATCH] fix(sse): estimate tokens from the object and count JSON length without building it (#7847) (#8558) Two changes with one root cause: several hot paths built a full JSON string only to read its .length, and one of them silently changed the answer. 1. CORRECTNESS -- combo's fallback-compression trigger estimateTokens(JSON.stringify(attemptBody)) took the STRING branch of estimateTokens, which is ceil(length / CHARS_PER_TOKEN) over the raw JSON. An inline base64 image is then charged as if every character of the data URL were prose. Measured on a 200 KB inline image: via string (before) 50,039 tokens via object (after) 1,231 tokens a 40x over-count, tripping fallback compression on requests nowhere near the context window. This is the same class #8368/#8401 fixed on the request path; the combo call site was missed. Passing the object routes through extractImageTokens, which charges images structurally. Text-only bodies are unaffected -- verified identical, and pinned by a test. 2. ALLOCATION -- jsonLength() Adds an exact serialized-length walker: same O(n) scan, no string. Used by estimateTokens' object branch and by streamReadinessPolicy (which runs on every streaming request and only ever used .length). Exactness matters because every consumer feeds a threshold, so this is property-tested against JSON.stringify over 4000 generated structures covering escaping, lone surrogates, omitted values, non-finite numbers, toJSON, Date, Map, cycles and BigInt. Anything outside the plain-JSON subset falls back to JSON.stringify for THAT SUBTREE only, so an exotic leaf never forces the message history back onto the allocating path. Honest scoping of the memory win: the string was always transient, and V8 collects it efficiently, so this is not 3 MiB of retained heap. Measured allocation churn over 20 calls on a 3.06 MiB body: 3.1 MiB -> 0.5 MiB, about 6x less. The #8549 benchmark row for this mechanism measures a HELD string and therefore overstates it; the correctness fix above is the larger deliverable here. --- .../fixes/7847-structural-json-size.md | 1 + open-sse/services/combo.ts | 3 +- open-sse/services/contextManager.ts | 12 +- open-sse/utils/jsonSize.ts | 127 ++++++++++++++++++ open-sse/utils/streamReadinessPolicy.ts | 6 +- ...combo-fallback-token-estimate-7847.test.ts | 92 +++++++++++++ tests/unit/json-size-exactness.test.ts | Bin 0 -> 6412 bytes 7 files changed, 236 insertions(+), 5 deletions(-) create mode 100644 changelog.d/fixes/7847-structural-json-size.md create mode 100644 open-sse/utils/jsonSize.ts create mode 100644 tests/unit/combo-fallback-token-estimate-7847.test.ts create mode 100644 tests/unit/json-size-exactness.test.ts diff --git a/changelog.d/fixes/7847-structural-json-size.md b/changelog.d/fixes/7847-structural-json-size.md new file mode 100644 index 0000000000..31bfe11f2e --- /dev/null +++ b/changelog.d/fixes/7847-structural-json-size.md @@ -0,0 +1 @@ +- fix(sse): estimate the combo fallback-compression trigger from the request object instead of `JSON.stringify(...)` (#7847) — the string path charged an inline base64 image as if every character were prose (~50k tokens instead of ~1.2k on a 200 KB image), falsely tripping compression on requests nowhere near the context window; the same over-count #8368/#8401 fixed elsewhere. Adds `jsonLength()`, an exact serialized-length walker (property-tested against `JSON.stringify`), and uses it for the readiness-timeout and token estimates so a multi-megabyte body is no longer materialized as a string just to be measured diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 23ad08cb34..b3aa368378 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -1730,7 +1730,8 @@ export async function handleComboChat({ config.fallbackCompressionMode !== "off" ) { const { estimateTokens } = await import("./contextManager.ts"); - const estimatedTokens = estimateTokens(JSON.stringify(attemptBody)); + // #7847: object, not JSON.stringify — the string branch mis-counts inline images. + const estimatedTokens = estimateTokens(attemptBody); if (estimatedTokens > (config.fallbackCompressionThreshold ?? 1000)) { const { applyCompression } = await import("./compression/strategySelector.ts"); const compressionResult = applyCompression( diff --git a/open-sse/services/contextManager.ts b/open-sse/services/contextManager.ts index 03d7cd6014..15b22ce0c9 100644 --- a/open-sse/services/contextManager.ts +++ b/open-sse/services/contextManager.ts @@ -7,6 +7,7 @@ import { REGISTRY } from "../config/providerRegistry.ts"; import { getModelContextLimit } from "../../src/lib/modelCapabilities.ts"; +import { jsonLength } from "../utils/jsonSize.ts"; // Default token limits per provider (fallbacks when not in registry) const DEFAULT_LIMITS: Record = { @@ -146,7 +147,10 @@ function extractImageTokens(node: unknown, seen: Set): { node: unknown; const record = node as Record; if (isInlineBase64ImageBlock(record)) { - return { node: { __image_token_estimate__: IMAGE_TOKEN_ESTIMATE }, tokens: IMAGE_TOKEN_ESTIMATE }; + return { + node: { __image_token_estimate__: IMAGE_TOKEN_ESTIMATE }, + tokens: IMAGE_TOKEN_ESTIMATE, + }; } let tokens = 0; @@ -173,8 +177,10 @@ export function estimateTokens(text: string | object | null | undefined): number return Math.ceil(text.length / CHARS_PER_TOKEN); } const { node, tokens: imageTokens } = extractImageTokens(text, new Set()); - const str = JSON.stringify(node); - return Math.ceil(str.length / CHARS_PER_TOKEN) + imageTokens; + // #7847: count the serialized length instead of building the string. Only `.length` was ever + // used, and on a multi-megabyte agent body that string is a pure transient allocation. + // jsonLength is exact (property-tested against JSON.stringify), so the estimate is unchanged. + return Math.ceil(jsonLength(node) / CHARS_PER_TOKEN) + imageTokens; } /** diff --git a/open-sse/utils/jsonSize.ts b/open-sse/utils/jsonSize.ts new file mode 100644 index 0000000000..ea714c7075 --- /dev/null +++ b/open-sse/utils/jsonSize.ts @@ -0,0 +1,127 @@ +/** + * Serialized JSON length without materializing the JSON (#7847). + * + * Several hot-path call sites only need `JSON.stringify(body).length` — a readiness-timeout + * threshold, a payload-size metric, a token estimate. On a 3.05 MiB agent request each of those + * allocates a full 3 MiB string that is read once for its length and thrown away, and #7847 + * reports that class of transient allocation driving V8/cgroup OOM under concurrent long-context + * traffic. + * + * `jsonLength()` walks the value and counts instead. Same O(n) scan, no allocation. + * + * It is EXACT, not an approximation: every consumer feeds a threshold, and an approximation + * would silently shift routing and timeout decisions. `tests/unit/json-size-exactness.test.ts` + * property-tests `jsonLength(x) === JSON.stringify(x).length` over generated structures. + * + * Anything outside the plain-JSON subset (Date, toJSON, class instances, Map, ...) falls back to + * `JSON.stringify` for THAT SUBTREE only, so an exotic leaf never forces the multi-megabyte + * message history back onto the allocating path. + */ + +/** Length of a JSON-encoded string, including the surrounding quotes. */ +function encodedStringLength(value: string): number { + let len = 2; // the quotes + for (let i = 0; i < value.length; i++) { + const code = value.charCodeAt(i); + if (code === 0x22 || code === 0x5c) { + len += 2; // \" and \\ + } else if (code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d) { + len += 2; // \b \t \n \f \r + } else if (code < 0x20) { + len += 6; // \u00XX + } else if (code >= 0xd800 && code <= 0xdfff) { + // Surrogates: a well-formed pair serializes as its two code units (2 chars); a LONE + // surrogate is escaped as \uXXXX since ES2019 well-formed JSON.stringify. + const isHigh = code <= 0xdbff; + const next = isHigh ? value.charCodeAt(i + 1) : NaN; + const paired = isHigh && next >= 0xdc00 && next <= 0xdfff; + if (paired) { + len += 2; + i++; // consume the low surrogate + } else { + len += 6; + } + } else { + len += 1; + } + } + return len; +} + +/** True for values JSON.stringify drops (object values) or renders as null (array items). */ +function isOmitted(value: unknown): boolean { + return value === undefined || typeof value === "function" || typeof value === "symbol"; +} + +function isPlainContainer(value: object): boolean { + if (Array.isArray(value)) return true; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + +/** + * Exact `JSON.stringify(value).length`, computed without building the string. + * Returns 0 for values JSON.stringify renders as `undefined` (functions, symbols, undefined), + * matching the `try { JSON.stringify(x).length } catch { 0 }` shape of the call sites replaced. + * Throws on circular structures and BigInt, exactly as JSON.stringify does. + */ +export function jsonLength(value: unknown): number { + return lengthOf(value, new Set()); +} + +function lengthOf(value: unknown, seen: Set): number { + if (value === null) return 4; // "null" + const type = typeof value; + + if (type === "string") return encodedStringLength(value as string); + if (type === "boolean") return value ? 4 : 5; + if (type === "number") { + // Non-finite numbers serialize as null. + return Number.isFinite(value as number) ? String(value).length : 4; + } + if (type === "bigint") { + // Match JSON.stringify, which throws rather than guessing an encoding. + throw new TypeError("Do not know how to serialize a BigInt"); + } + if (isOmitted(value)) return 0; + if (type !== "object") return 0; + + const obj = value as object; + + // Delegate anything that is not a plain object/array — Date, class instances with toJSON, + // Map, boxed primitives. Scoped to this subtree so the big arrays stay on the fast path. + if (!isPlainContainer(obj) || typeof (obj as { toJSON?: unknown }).toJSON === "function") { + const encoded = JSON.stringify(obj); + return encoded === undefined ? 0 : encoded.length; + } + + if (seen.has(obj)) { + throw new TypeError("Converting circular structure to JSON"); + } + seen.add(obj); + try { + if (Array.isArray(obj)) { + let len = 2; // [] + for (let i = 0; i < obj.length; i++) { + if (i > 0) len += 1; // comma + const item = obj[i]; + // Omitted values render as null inside arrays rather than disappearing. + len += isOmitted(item) ? 4 : lengthOf(item, seen); + } + return len; + } + + let len = 2; // {} + let first = true; + for (const key of Object.keys(obj)) { + const item = (obj as Record)[key]; + if (isOmitted(item)) continue; // the whole entry disappears + if (!first) len += 1; // comma + first = false; + len += encodedStringLength(key) + 1 + lengthOf(item, seen); // "key":value + } + return len; + } finally { + seen.delete(obj); + } +} diff --git a/open-sse/utils/streamReadinessPolicy.ts b/open-sse/utils/streamReadinessPolicy.ts index 9dcf631345..2dcc374809 100644 --- a/open-sse/utils/streamReadinessPolicy.ts +++ b/open-sse/utils/streamReadinessPolicy.ts @@ -1,3 +1,4 @@ +import { jsonLength } from "./jsonSize.ts"; import { getRegistryEntry } from "../config/providerRegistry.ts"; type StreamReadinessBody = Record | null | undefined; @@ -31,7 +32,10 @@ function countArrayField(body: StreamReadinessBody, field: "input" | "messages" function estimateBodyChars(body: StreamReadinessBody): number { if (!body) return 0; try { - return JSON.stringify(body).length; + // #7847: count the serialized length without building the string — this runs on every + // streaming request, and only `.length` was ever used. jsonLength is exact (property-tested + // against JSON.stringify), so the readiness thresholds are unchanged. + return jsonLength(body); } catch { return 0; } diff --git a/tests/unit/combo-fallback-token-estimate-7847.test.ts b/tests/unit/combo-fallback-token-estimate-7847.test.ts new file mode 100644 index 0000000000..1a8e57838c --- /dev/null +++ b/tests/unit/combo-fallback-token-estimate-7847.test.ts @@ -0,0 +1,92 @@ +// combo's fallback-compression trigger must estimate tokens from the request OBJECT (#7847). +// +// It used to call `estimateTokens(JSON.stringify(attemptBody))`, which takes the string branch of +// estimateTokens — `ceil(length / CHARS_PER_TOKEN)` over the raw JSON. An inline base64 image is +// then charged as if every character of the data URL were prose, the same over-count #8368/#8401 +// fixed on the request path. On a 200 KB inline image that read ~50k tokens instead of ~1.2k, +// tripping fallback compression on a request nowhere near the context window. +// +// Passing the object instead routes through extractImageTokens, which charges images structurally. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-token-estimate-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const { estimateTokens } = await import("../../open-sse/services/contextManager.ts"); +const core = await import("../../src/lib/db/core.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = ORIGINAL_DATA_DIR; +}); + +const imageBody = (base64Chars: number) => ({ + model: "claude-opus-5", + messages: [ + { + role: "user", + content: [ + { type: "text", text: "describe this screenshot" }, + { + type: "image_url", + image_url: { url: `data:image/png;base64,${"A".repeat(base64Chars)}` }, + }, + ], + }, + ], +}); + +const textBody = () => ({ + model: "claude-opus-5", + max_tokens: 100, + messages: [{ role: "user", content: "hello world ".repeat(500) }], + tools: [{ type: "function", function: { name: "t", description: "does a thing" } }], +}); + +test("an inline image is charged structurally, not as raw data-URL text", () => { + const small = estimateTokens(imageBody(10_000)); + const large = estimateTokens(imageBody(200_000)); + + // A 20x larger base64 payload must not cost 20x the tokens — the image is charged as an image. + assert.ok( + large < small * 2, + `estimate scaled with the base64 length (${small} -> ${large}); the data URL is being counted as text` + ); + assert.ok(large < 10_000, `expected a bounded image charge, got ${large} tokens`); +}); + +test("the string path is what over-counts — this is why the call site must pass the object", () => { + const body = imageBody(200_000); + const viaObject = estimateTokens(body); + const viaString = estimateTokens(JSON.stringify(body)); + + assert.ok( + viaString > viaObject * 10, + `expected the string path to over-count heavily (object=${viaObject}, string=${viaString}) — ` + + "if this ever stops being true, the regression guard below is measuring nothing" + ); +}); + +test("text-only bodies are unaffected: object and string paths agree", () => { + const body = textBody(); + assert.equal( + estimateTokens(body), + estimateTokens(JSON.stringify(body)), + "the switch to the object path must be a no-op for the common text-only request" + ); +}); + +test("estimateTokens still handles the plain shapes", () => { + assert.equal(estimateTokens(null), 0); + assert.equal(estimateTokens(undefined), 0); + assert.equal(estimateTokens(""), 0); + assert.equal(estimateTokens("abcd"), 1); + assert.ok(estimateTokens({ a: "x".repeat(400) }) > 0); +}); diff --git a/tests/unit/json-size-exactness.test.ts b/tests/unit/json-size-exactness.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..dc582bb09d253cb48fbd60deb682e38095040026 GIT binary patch literal 6412 zcmbtYOK%&=5l*i96RAu#@Nmt?kfvmtYuT`Oy$c7i7O-UGAqi4*rb&)DoaRo?h~m-& zj9`HvhXe?+2ofNGZ`tf2hunf(^C!GlBgol5AzyXR42KeB?;?g^k~7uS_4xX$s&2LD znO5;nWjstrZjC0HPDy^2iHLr(|LI;+r%4zO!@=CGH6tTL@?7|7G$;2*H*a-s)|y+b z7XIkrnM~&7t5|0fnb1JWK$Dd`TiE5r}CTo=Hli2*OxuO=&ohDog7S z_tj*I1v*qQVe8>gCV0&Hr(zx{5j1r8xul82OTS6?p>H}()Ok1&DX*ES3|r|Cwz7?S z6b@2Kpd4OMApMXv>pE#gjvpy4S+$H+HXJz=YKm1#!uRDg75zxU-T)rpIsUNCR3_=X zkxHG)fD)08;AlF6Q+le>hCh=2SO(3la57a%N(*91TGAj<6LMk|$R0m&J}4xF*3eje zZ=bX{>^=)_aX?wsDoaqfOJXL%lX(`>fz2lL_vG`8Svq+7gVAHiy-+NqVAO!lC_ zJrhwTdz8iFSk2-(MWQdGdp)uluhH&ZT5OR?C=hWzl|DPYdl42!4b&4^)0+l7E`C$R zX1iOhZXU0c?e1d#Kn?(dfbNk;y>h1OD^)qAUZ<*5 zN1-oWuTJfbS6iOK9GJdDaDal>U%tQ}eI!$vOu{(SY3S2X#uACfW^SzLF;d=4017qf z0jz^ajOQgTY_rzJX`t4_%Fu(W5w<^kq`(5e^+fZGW1GM=~Xi zK(id(y?d8Dj&35;EQxbzm*3;R_o=?jyMc#Sa%m2a8 z0!@4O_YZz?vVZX8(cUNf&~ym>j>DmLrjZC^^Yz+DXmUQw6d>zdGcWtXA0Hj*Na&H3 zj^*qqj*z{h6zAnAIqDw`?3?d>{|5}6G*OX#kj0^o13<6;@(23&Kj=UI{QH0Z_-lIg z)i+;!^Tn%wz53cp$%%Rv(wmom{?(h8zx&Obm%snp>leSp$Jh4bEBoR`xeI;wy(XNL+CAfXW;gSD5qF-uG!uAZLmi{A1Et z)49EUL1~_atCi*gZ&P}+)M@Qu8|G?MiMpzb;snh)OtTj#JtI+%UzHy{Qw3n>p5g{1m&2a z2DNKO0dH@M)?4?nA}Gu~dLn(51b1xVsTURLF08bUGSvenD0g_|eSnWU6yxLi^>x3- za$dm8;IE64vWAqavz~KdnRV2R)A4(YaCvGY4Lc8^{uuf<_{}oxcAgIi8?xnE?>a@> z(5sX~+hOU8wX|%p6nZJ@g1C$N0}?`bG9NVnY+R&ZQHB~oglSw|ymQ|hp!qVn*c3Td zyCHa{j#u65=5VlyS{2%hMz0!xO)bEcO#qdZo_4=&8$fjG3oTLi!zGoS0QZ%7GxL8m z1RXDL2uw@B`CK9ZTSX4TNyts>dhAVjZG6s14d{^1c^!RgChIg1k;X@}*<9oQ%99*) z$2HqhyJsfiYi-o3>vKznsMX24ED0#A#kydMyyx=p{_>)V`hrBxom##@#eW6wx%)F= z);$W)3QzGnj7jyMp)*8B&A#&k!Mw2^C)0C+MFh8-dAnDqevjI9@{1C*0JEST6?TBk z>V*EZLu?151yhH6SXqz7g_HI5Wu4>7k_D+anty6Ns2FgCUS3-9+>#PW?Taao5T^N* zn)>rpnjVzDtR2V-C^Um&je!cnvfgN8X5rg{4)rnm@7%EB@gZWnAk0ZffO%`3Tf=9p z;)dzm@(eNfBI0(9Qgz|L8mlq1pvlboX(FYG&zcKGzGfpCgY)H#K0-g`IvuaG)9`LK zI@<@H*E4@ju*ol-+FCFd zOAUq7;sgrx!1o?Z7hIc+Q@A6L6ACan8BZ$HP`N7sVF`A@_z6#NbgXrjV3Ga2kq8-H zhD%EkOHzOoXNNHpCVVpZh2PnI#!L5Vzz0!KmmY@?43pYRPFC+LkgIvoqz`fP2u}%-8zX=z?m@88NQx<{xYfUc5s4w* zWAO!itU7pGILvu4T@zOh8_=QuF;7OrNcBZ@fXl_^&N!u3S@*29qo<6*4PH@GP%%5_Z#+5dw_o48x$$O5_&Yl00BCLV(HSf-BJ