mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-11 01:32:22 +03:00
fix(logging): use configurable max-depth when bounding logged tool_calls
requestLogger.ts's cloneBoundedForLog had its own hardcoded depth cap of 6,
independent of the existing configurable getChatLogMaxDepth(). A typical
Chat Completions response body's responseBody.choices[0].message.tool_calls[0].function
sits at exactly depth 6, so every logged tool call's function field
(name+arguments) was silently replaced with the literal string "[MaxDepth]"
before ever being stored — corrupting the data, not just how it renders.
Bumped the shared default 6->20 and switched requestLogger.ts to read it
instead of using its own literal.
(cherry picked from commit a2df6cf289)
This commit is contained in:
committed by
diegosouzapw
parent
aae408f585
commit
f617fd0035
@@ -1,4 +1,5 @@
|
||||
import { getPendingById } from "@/lib/usage/usageHistory";
|
||||
import { getChatLogMaxDepth } from "@/lib/logEnv";
|
||||
import { sanitizeErrorMessage } from "./error.ts";
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
@@ -148,7 +149,7 @@ export function cloneBoundedForLog(value: unknown, depth = 0, key: string | null
|
||||
if (ArrayBuffer.isView(value)) {
|
||||
return `[binary ${(value as ArrayBufferView).byteLength} bytes]`;
|
||||
}
|
||||
if (depth >= 6) return "[MaxDepth]";
|
||||
if (depth >= getChatLogMaxDepth()) return "[MaxDepth]";
|
||||
|
||||
if (Array.isArray(value)) {
|
||||
// Idempotence (#7847): an already-bounded array is [marker, ...tail] — MAX_LOG_ARRAY_ITEMS + 1
|
||||
|
||||
@@ -150,8 +150,17 @@ export function getChatLogArrayTailItems(): number {
|
||||
return parsePositiveInt(process.env.CHAT_LOG_ARRAY_TAIL_ITEMS, 24);
|
||||
}
|
||||
|
||||
/**
|
||||
* Was a hardcoded 6 — trivially too shallow for real Chat Completions tool
|
||||
* calls: `body.choices[0].message.tool_calls[0].function` alone is already
|
||||
* 6 levels deep (body→choices→[i]→message→tool_calls→[i]→function), so
|
||||
* EVERY logged tool call got its `function` field (name + arguments)
|
||||
* replaced outright with the literal string "[MaxDepth]" before the name/
|
||||
* arguments one level further in were ever reached — not an edge case, a
|
||||
* universal truncation of tool-call data in call log artifacts.
|
||||
*/
|
||||
export function getChatLogMaxDepth(): number {
|
||||
return parsePositiveInt(process.env.CHAT_LOG_MAX_DEPTH, 6);
|
||||
return parsePositiveInt(process.env.CHAT_LOG_MAX_DEPTH, 20);
|
||||
}
|
||||
|
||||
export function getChatLogMaxObjectKeys(): number {
|
||||
|
||||
@@ -38,6 +38,40 @@ test("cloneBoundedForLog: nested tools field still exempt", () => {
|
||||
assert.equal(result.body.tools.length, 30);
|
||||
});
|
||||
|
||||
// Regression: a Chat Completions response's tool_calls[].function is 6 levels
|
||||
// deep from the response body (body -> choices -> [i] -> message -> tool_calls
|
||||
// -> [i] -> function) — the depth cap used to be a hardcoded 6, so every
|
||||
// logged tool call's `function` (name + arguments) got replaced outright with
|
||||
// the literal string "[MaxDepth]", not just deeply truncated. This broke tool
|
||||
// call rendering in the request-detail view for ANY response with a tool
|
||||
// call — not an edge case, universal.
|
||||
test("cloneBoundedForLog: tool_calls[].function survives at its natural depth (was clobbered to '[MaxDepth]')", () => {
|
||||
const body = {
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
message: {
|
||||
role: "assistant",
|
||||
content: null,
|
||||
tool_calls: [
|
||||
{
|
||||
id: "call_1",
|
||||
type: "function",
|
||||
function: { name: "write", arguments: '{"path":"/tmp/x","content":"hi"}' },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const result = cloneBoundedForLog(body) as {
|
||||
choices: Array<{ message: { tool_calls: Array<{ function: unknown }> } }>;
|
||||
};
|
||||
const fn = result.choices[0].message.tool_calls[0].function;
|
||||
assert.notEqual(fn, "[MaxDepth]", "function must not be clobbered to the MaxDepth placeholder");
|
||||
assert.deepEqual(fn, { name: "write", arguments: '{"path":"/tmp/x","content":"hi"}' });
|
||||
});
|
||||
|
||||
test("cloneBoundedForLog: top-level array without key context still truncated", () => {
|
||||
const arr = Array.from({ length: 45 }, (_, i) => i);
|
||||
const result = cloneBoundedForLog(arr) as unknown[];
|
||||
|
||||
Reference in New Issue
Block a user