mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 13:52:09 +03:00
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.
This commit is contained in:
1
changelog.d/fixes/7847-structural-json-size.md
Normal file
1
changelog.d/fixes/7847-structural-json-size.md
Normal file
@@ -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
|
||||
@@ -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(
|
||||
|
||||
@@ -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<string, number> = {
|
||||
@@ -146,7 +147,10 @@ function extractImageTokens(node: unknown, seen: Set<unknown>): { node: unknown;
|
||||
|
||||
const record = node as Record<string, unknown>;
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
127
open-sse/utils/jsonSize.ts
Normal file
127
open-sse/utils/jsonSize.ts
Normal file
@@ -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<object>());
|
||||
}
|
||||
|
||||
function lengthOf(value: unknown, seen: Set<object>): 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<string, unknown>)[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);
|
||||
}
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
import { jsonLength } from "./jsonSize.ts";
|
||||
import { getRegistryEntry } from "../config/providerRegistry.ts";
|
||||
|
||||
type StreamReadinessBody = Record<string, unknown> | 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;
|
||||
}
|
||||
|
||||
92
tests/unit/combo-fallback-token-estimate-7847.test.ts
Normal file
92
tests/unit/combo-fallback-token-estimate-7847.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
BIN
tests/unit/json-size-exactness.test.ts
Normal file
BIN
tests/unit/json-size-exactness.test.ts
Normal file
Binary file not shown.
Reference in New Issue
Block a user