fix(core): preserve usage and cost metadata on structured output failures#1163
fix(core): preserve usage and cost metadata on structured output failures#1163
Conversation
🦋 Changeset detectedLatest commit: a2b2017 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying voltagent with
|
| Latest commit: |
a2b2017
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7812c373.voltagent.pages.dev |
| Branch Preview URL: | https://fix-structured-output-cost-m.voltagent.pages.dev |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughAgent.generateText now preserves and propagates resolved usage, finish reason, and provider cost/metadata when structured-output generation fails, exposing this metadata via VoltAgentError.metadata and including it in observability traces and error hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Agent
participant Model
participant Tracing
participant Hooks
Client->>Agent: generateText(output: structured)
Agent->>Model: doGenerate(...)
Model-->>Agent: success (finishReason: tool-calls, providerMetadata, usage)
Agent->>Agent: ensureStructuredOutputGenerated (await)
alt no structured output produced
Agent->>Tracing: end llm span with error + usage/finishReason/providerMetadata
Agent->>Tracing: set root span usage/finishReason/providerMetadata
Agent->>Hooks: onError(error with VoltAgentError.metadata)
Agent->>Hooks: onEnd(error payload with metadata)
Agent-->>Client: reject with VoltAgentError
else structured output produced
Agent-->>Client: resolve with structured output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/agent/agent.spec.ts (1)
939-946: Also assert preserved metadata ononErrorpayload.
onEndvalidates metadata propagation, butonErrorcurrently checks onlycode/stage. Adding metadata checks here will guard the full error-hook contract.Suggested assertion hardening
expect(onError).toHaveBeenCalledWith( expect.objectContaining({ error: expect.objectContaining({ code: "STRUCTURED_OUTPUT_NOT_GENERATED", stage: "response_parsing", + metadata: expect.objectContaining({ + finishReason: "tool-calls", + usage: expect.objectContaining({ + inputTokens: 12, + outputTokens: 6, + totalTokens: 18, + }), + providerMetadata: expect.objectContaining({ + openrouter: expect.objectContaining({ + usage: expect.objectContaining({ + cost: 0.0012, + isByok: true, + }), + }), + }), + }), }), }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/agent.spec.ts` around lines 939 - 946, The test currently asserts only code/stage on the onError payload; update the expectation for onError (the expect(onError).toHaveBeenCalledWith(...) block) to also assert that metadata is preserved by adding an objectContaining for the metadata field (e.g., expect.objectContaining({ metadata: expect.objectContaining({ ...the same keys/assertions used in the onEnd check... }) })). Locate the onError assertion in agent.spec.ts and mirror the metadata assertions used in the onEnd validation so the error hook contract is fully covered.packages/core/src/agent/agent-observability.spec.ts (1)
327-339: Assert full provider cost metadata on failure-path spans.This test currently checks
usage.coston LLM and root spans, but not all propagated fields. Addusage.is_byokandusage.cost_details.*assertions to lock in complete metadata preservation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/agent-observability.spec.ts` around lines 327 - 339, Update the test to assert that the full provider cost metadata is preserved on failure-path spans by adding assertions for usage.is_byok and usage.cost_details on both the LLM span (llmSpan) and the root agent span (rootSpan) — specifically check that llmSpan.attributes["usage.is_byok"] is true and llmSpan.attributes["usage.cost_details"] matches the expected cost_details object (and likewise for rootSpan.attributes["usage.is_byok"] and rootSpan.attributes["usage.cost_details"]) so the test locks in complete metadata preservation across spans.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/agent/agent-observability.spec.ts`:
- Around line 271-274: Replace the untyped events: any[] with a concrete event
type: define or import a WebSocketEvent (or existing event interface used by
WebSocketEventEmitter) and declare events as WebSocketEvent[]; also type the
callback parameter in
WebSocketEventEmitter.getInstance().onWebSocketEvent((event) => ...) to accept
WebSocketEvent so the pushed values are type-checked against the emitter's event
shape (reference symbols: events, WebSocketEventEmitter.getInstance(),
onWebSocketEvent).
---
Nitpick comments:
In `@packages/core/src/agent/agent-observability.spec.ts`:
- Around line 327-339: Update the test to assert that the full provider cost
metadata is preserved on failure-path spans by adding assertions for
usage.is_byok and usage.cost_details on both the LLM span (llmSpan) and the root
agent span (rootSpan) — specifically check that
llmSpan.attributes["usage.is_byok"] is true and
llmSpan.attributes["usage.cost_details"] matches the expected cost_details
object (and likewise for rootSpan.attributes["usage.is_byok"] and
rootSpan.attributes["usage.cost_details"]) so the test locks in complete
metadata preservation across spans.
In `@packages/core/src/agent/agent.spec.ts`:
- Around line 939-946: The test currently asserts only code/stage on the onError
payload; update the expectation for onError (the
expect(onError).toHaveBeenCalledWith(...) block) to also assert that metadata is
preserved by adding an objectContaining for the metadata field (e.g.,
expect.objectContaining({ metadata: expect.objectContaining({ ...the same
keys/assertions used in the onEnd check... }) })). Locate the onError assertion
in agent.spec.ts and mirror the metadata assertions used in the onEnd validation
so the error hook contract is fully covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa53e0e1-f315-4203-9fd2-39806073377e
📒 Files selected for processing (4)
.changeset/soft-cost-errors.mdpackages/core/src/agent/agent-observability.spec.tspackages/core/src/agent/agent.spec.tspackages/core/src/agent/agent.ts
| const events: any[] = []; | ||
| const unsubscribe = WebSocketEventEmitter.getInstance().onWebSocketEvent((event) => { | ||
| events.push(event); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `any[]` event declarations in observability specs
rg -n --type=ts 'const\s+events\s*:\s*any\[\]\s*=' packages/core/src/agent/agent-observability.spec.tsRepository: VoltAgent/voltagent
Length of output: 421
Replace any[] with a typed event shape to maintain type safety.
Line 271 uses events: any[], which violates the TypeScript-first codebase requirement. Define a specific event type instead:
Typed alternative
+ type ObservabilityEvent = {
+ type: string;
+ span?: {
+ name?: string;
+ attributes: Record<string, unknown>;
+ status?: { code: SpanStatusCode };
+ };
+ };
- const events: any[] = [];
+ const events: ObservabilityEvent[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/agent/agent-observability.spec.ts` around lines 271 - 274,
Replace the untyped events: any[] with a concrete event type: define or import a
WebSocketEvent (or existing event interface used by WebSocketEventEmitter) and
declare events as WebSocketEvent[]; also type the callback parameter in
WebSocketEventEmitter.getInstance().onWebSocketEvent((event) => ...) to accept
WebSocketEvent so the pushed values are type-checked against the emitter's event
shape (reference symbols: events, WebSocketEventEmitter.getInstance(),
onWebSocketEvent).
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
When
generateTextgets a successful model response but fails to produce structured output,VoltAgent throws a structured output error without preserving the resolved usage or
provider-reported cost metadata on the error path.
What is the new behavior?
Structured output failures after a successful model response now preserve usage, finish reason,
and provider metadata in
VoltAgentError.metadata, and the same provider cost metadata is kepton the failed LLM/root observability spans.
fixes (issue)
N/A
Notes for reviewers
agent.spec.tsandagent-observability.spec.ts.pnpm vitest run src/agent/agent.spec.ts src/agent/agent-observability.spec.tsSummary by cubic
Preserves usage, finish reason, and provider cost when structured output fails after a successful model call in
@voltagent/core. Also fixes the structured output check to be async so spans and hooks record the correct cost.Bug Fixes
VoltAgentError.metadata).onEnd/onErrorhooks. Tests cover failure path and observability.Dependencies
tsxto 4.21.0 across workspaces (no runtime changes).Written for commit a2b2017. Summary will update on new commits.
Summary by CodeRabbit