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 0000000000..dc582bb09d Binary files /dev/null and b/tests/unit/json-size-exactness.test.ts differ