-
-
Notifications
You must be signed in to change notification settings - Fork 714
fix(core): tighten prompt context usage telemetry #1172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "@voltagent/core": patch | ||
| --- | ||
|
|
||
| fix: tighten prompt-context usage telemetry | ||
|
|
||
| - redact nested large binary fields when estimating prompt context usage | ||
| - exclude runtime-only tool metadata from tool schema token estimates | ||
| - avoid emitting cached and reasoning token span attributes when their values are zero |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -68,4 +68,86 @@ describe("prompt context usage estimation", () => { | |||||||||
| "usage.prompt_context.tool_count": 2, | ||||||||||
| }); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| it("sanitizes nested binary args recursively and ignores provider-only metadata", () => { | ||||||||||
| const circularArgsA: Record<string, unknown> = { | ||||||||||
| content: { | ||||||||||
| metadata: { | ||||||||||
| data: "x".repeat(8_000), | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| attachments: [{ image: "y".repeat(8_000) }], | ||||||||||
| }; | ||||||||||
| circularArgsA.self = circularArgsA; | ||||||||||
|
|
||||||||||
| const circularArgsB: Record<string, unknown> = { | ||||||||||
| content: { | ||||||||||
| metadata: { | ||||||||||
| data: "short", | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| attachments: [{ image: "tiny" }], | ||||||||||
| }; | ||||||||||
| circularArgsB.self = circularArgsB; | ||||||||||
|
|
||||||||||
| const toolAEstimate = estimatePromptContextUsage({ | ||||||||||
| tools: { | ||||||||||
| searchDocs: { | ||||||||||
| description: "Search the documentation", | ||||||||||
| inputSchema: z.object({ query: z.string() }), | ||||||||||
| outputSchema: z.object({ answer: z.string() }), | ||||||||||
| providerOptions: { | ||||||||||
| openai: { | ||||||||||
| metadata: "provider-only".repeat(2_000), | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| needsApproval: true, | ||||||||||
| args: circularArgsA, | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| const toolBEstimate = estimatePromptContextUsage({ | ||||||||||
| tools: { | ||||||||||
| searchDocs: { | ||||||||||
| description: "Search the documentation", | ||||||||||
| inputSchema: z.object({ query: z.string() }), | ||||||||||
| outputSchema: z.object({ answer: z.string() }), | ||||||||||
| providerOptions: { | ||||||||||
| openai: { | ||||||||||
| metadata: "ignored", | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| needsApproval: false, | ||||||||||
| args: circularArgsB, | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| expect(toolAEstimate?.toolTokensEstimated).toBeGreaterThan(0); | ||||||||||
| expect(toolAEstimate?.toolTokensEstimated).toBe(toolBEstimate?.toolTokensEstimated); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| it("ignores non-plain args values when estimating tool tokens", () => { | ||||||||||
| const withArrayArgs = estimatePromptContextUsage({ | ||||||||||
| tools: { | ||||||||||
| searchDocs: { | ||||||||||
| description: "Search the documentation", | ||||||||||
| inputSchema: z.object({ query: z.string() }), | ||||||||||
| args: ["x".repeat(10_000)], | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| const withoutArgs = estimatePromptContextUsage({ | ||||||||||
| tools: { | ||||||||||
| searchDocs: { | ||||||||||
| description: "Search the documentation", | ||||||||||
| inputSchema: z.object({ query: z.string() }), | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| expect(withArrayArgs?.toolTokensEstimated).toBe(withoutArgs?.toolTokensEstimated); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This equality-only assertion can pass when both estimates are undefined, so the test may miss regressions in token estimation. Prompt for AI agents
Suggested change
|
||||||||||
| }); | ||||||||||
| }); | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ const BINARY_PART_TYPES = new Set([ | |
| "media", | ||
| ]); | ||
| const LARGE_BINARY_KEYS = new Set(["audio", "base64", "bytes", "data", "image"]); | ||
| const CIRCULAR_REFERENCE_PLACEHOLDER = "[circular]"; | ||
|
|
||
| type PromptMessage = { | ||
| role?: string; | ||
|
|
@@ -164,12 +165,25 @@ function serializePromptValue(value: unknown): string { | |
| } | ||
|
|
||
| function sanitizeRecord(record: Record<string, unknown>): Record<string, unknown> { | ||
| return sanitizeRecordValue(record, new Set<object>()); | ||
| } | ||
|
|
||
| function sanitizeRecordValue( | ||
| record: Record<string, unknown>, | ||
| seen: Set<object>, | ||
| ): Record<string, unknown> { | ||
| if (seen.has(record)) { | ||
| return { circular: CIRCULAR_REFERENCE_PLACEHOLDER }; | ||
| } | ||
|
|
||
| seen.add(record); | ||
| const sanitized: Record<string, unknown> = {}; | ||
|
|
||
| for (const [key, value] of Object.entries(record)) { | ||
| sanitized[key] = LARGE_BINARY_KEYS.has(key) ? "[omitted]" : value; | ||
| sanitized[key] = LARGE_BINARY_KEYS.has(key) ? "[omitted]" : sanitizeValue(value, seen); | ||
| } | ||
|
|
||
| seen.delete(record); | ||
|
Comment on lines
+168
to
+186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Circular protection still misses recursive Line 157 recursively calls Proposed fix-function serializePromptValue(value: unknown): string {
+function serializePromptValue(value: unknown, seen: Set<object> = new Set()): string {
if (typeof value === "string") {
return value;
}
if (typeof value === "number" || typeof value === "boolean") {
return String(value);
}
if (Array.isArray(value)) {
- return value
- .map((entry) => serializePromptValue(entry))
- .filter((entry) => entry.trim().length > 0)
- .join("\n");
+ if (seen.has(value)) return CIRCULAR_REFERENCE_PLACEHOLDER;
+ seen.add(value);
+ try {
+ return value
+ .map((entry) => serializePromptValue(entry, seen))
+ .filter((entry) => entry.trim().length > 0)
+ .join("\n");
+ } finally {
+ seen.delete(value);
+ }
}
if (!value || typeof value !== "object") {
return "";
}
+ if (seen.has(value as object)) return CIRCULAR_REFERENCE_PLACEHOLDER;
+ seen.add(value as object);
- const record = value as Record<string, unknown>;
- const type = typeof record.type === "string" ? record.type : undefined;
+ try {
+ const record = value as Record<string, unknown>;
+ const type = typeof record.type === "string" ? record.type : undefined;
- if (typeof record.text === "string") {
- return record.text;
- }
+ if (typeof record.text === "string") {
+ return record.text;
+ }
- if (type && BINARY_PART_TYPES.has(type)) {
- return `[${type}]`;
- }
+ if (type && BINARY_PART_TYPES.has(type)) {
+ return `[${type}]`;
+ }
- if (type === "tool-call") {
- const toolName = typeof record.toolName === "string" ? record.toolName : "tool";
- const input = serializePromptValue(record.input);
- return input ? `tool-call ${toolName}: ${input}` : `tool-call ${toolName}`;
- }
+ if (type === "tool-call") {
+ const toolName = typeof record.toolName === "string" ? record.toolName : "tool";
+ const input = serializePromptValue(record.input, seen);
+ return input ? `tool-call ${toolName}: ${input}` : `tool-call ${toolName}`;
+ }
- if (type === "tool-result") {
- const toolName = typeof record.toolName === "string" ? record.toolName : "tool";
- const output = serializePromptValue(record.output);
- return output ? `tool-result ${toolName}: ${output}` : `tool-result ${toolName}`;
- }
+ if (type === "tool-result") {
+ const toolName = typeof record.toolName === "string" ? record.toolName : "tool";
+ const output = serializePromptValue(record.output, seen);
+ return output ? `tool-result ${toolName}: ${output}` : `tool-result ${toolName}`;
+ }
- if ("content" in record) {
- const nestedContent = serializePromptValue(record.content);
- if (nestedContent) {
- return nestedContent;
+ if ("content" in record) {
+ const nestedContent = serializePromptValue(record.content, seen);
+ if (nestedContent) {
+ return nestedContent;
+ }
}
- }
- return safeStringify(sanitizeRecord(record));
+ return safeStringify(sanitizeRecord(record));
+ } finally {
+ seen.delete(value as object);
+ }
}🤖 Prompt for AI Agents |
||
| return sanitized; | ||
| } | ||
|
|
||
|
|
@@ -200,9 +214,7 @@ function serializeToolDefinition(name: string, tool: unknown): Record<string, un | |
| outputSchema: normalizeSchema(candidate.outputSchema ?? candidate.output_schema), | ||
| } | ||
| : {}), | ||
| ...(candidate.providerOptions ? { providerOptions: candidate.providerOptions } : {}), | ||
| ...(candidate.args ? { args: sanitizeRecord(candidate.args as Record<string, unknown>) } : {}), | ||
| ...(candidate.needsApproval !== undefined ? { needsApproval: candidate.needsApproval } : {}), | ||
| ...(isPlainObject(candidate.args) ? { args: sanitizeRecord(candidate.args) } : {}), | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -221,3 +233,43 @@ function normalizeSchema(schema: unknown): unknown { | |
|
|
||
| return schema; | ||
| } | ||
|
|
||
| function sanitizeValue(value: unknown, seen: Set<object>): unknown { | ||
| if (value === null || value === undefined) { | ||
| return value; | ||
| } | ||
|
|
||
| if (typeof value !== "object") { | ||
| return value; | ||
| } | ||
|
|
||
| if (value instanceof Date || value instanceof RegExp) { | ||
| return value; | ||
| } | ||
|
|
||
| if (Array.isArray(value)) { | ||
| if (seen.has(value)) { | ||
| return [CIRCULAR_REFERENCE_PLACEHOLDER]; | ||
| } | ||
|
|
||
| seen.add(value); | ||
| const sanitized = value.map((entry) => sanitizeValue(entry, seen)); | ||
| seen.delete(value); | ||
| return sanitized; | ||
| } | ||
|
|
||
| if (!isPlainObject(value)) { | ||
| return value; | ||
| } | ||
|
|
||
| return sanitizeRecordValue(value, seen); | ||
| } | ||
|
|
||
| function isPlainObject(value: unknown): value is Record<string, unknown> { | ||
| if (!value || typeof value !== "object" || Array.isArray(value)) { | ||
| return false; | ||
| } | ||
|
|
||
| const prototype = Object.getPrototypeOf(value); | ||
| return prototype === Object.prototype || prototype === null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 5900
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 658
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 2286
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 313
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 843
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 460
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 957
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 45
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 2179
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 151
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 913
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 200
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 1258
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 87
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 1160
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 93
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 229
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 334
🏁 Script executed:
Repository: VoltAgent/voltagent
Length of output: 527
Import and use
ObservabilityWebSocketEventto maintain type safety.Line 216 uses
any[], which defeats TypeScript's compile-time safety. Replace with the proper event type already exported from the observability module:Add
ObservabilityWebSocketEventto the import from"../observability":Type the
eventsarray and callback parameter:The
ObservabilityWebSocketEventinterface is already defined and exported; no need for a local type definition. This applies to all similar instances in this test file.As per coding guidelines:
**/*.ts— Maintain type safety in TypeScript-first codebase.🤖 Prompt for AI Agents