fix(agents): pass through full AI SDK StepResult in onStepFinish#55
Merged
zrosenbauer merged 8 commits intomainfrom Mar 25, 2026
Merged
fix(agents): pass through full AI SDK StepResult in onStepFinish#55zrosenbauer merged 8 commits intomainfrom
zrosenbauer merged 8 commits intomainfrom
Conversation
…nish
StepFinishEvent is now a superset of the Vercel AI SDK's
StepResult<ToolSet>. All SDK fields (text, toolCalls, toolResults,
finishReason, usage, reasoning, sources, response, etc.) are passed
through unchanged instead of being stripped to summary fields.
- Remove stripping of toolCalls to { toolName, argsTextLength }
- Remove stripping of toolResults to { toolName, resultTextLength }
- Remove extractUsage/safeSerializedLength/extractProperty helpers
- Export new AIStepResult type alias
- Update streaming example to use AI SDK field names
- Update docs to reflect new StepFinishEvent shape
- Delete ISSUES.md (all issues closed)
Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3d16940 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStep lifecycle and result/event types in Changes
Sequence Diagram(s)sequenceDiagram
participant FlowEngine as Flow Engine
participant FlowStep as Flow Step / StepBuilder
participant Agent as Agent Runtime
participant AISDK as AI SDK
participant Consumer as Consumer / Hook
FlowEngine->>FlowStep: execute step (create stepId, stepOperation)
FlowStep->>Agent: run agent step (if stepOperation == "agent")
Agent->>AISDK: invoke model/tool loop
AISDK-->>Agent: return StepResult<ToolSet> (full AIStepResult)
Agent->>FlowStep: forward AIStepResult (passthrough)
FlowStep->>Consumer: emit onStepFinish (StepFinishEvent = AIStepResult + {stepId, stepOperation, agentChain})
Consumer-->>FlowEngine: consume FlowStepResult.output / duration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/step-finish-ai-sdk-passthrough.md:
- Around line 1-7: The changeset front matter currently sets "@funkai/agents":
minor but the body describes breaking API changes; update the front matter to
bump "@funkai/agents" to major so the release type matches the breaking
changes—edit the front-matter line `"@funkai/agents": minor` to
`"@funkai/agents": major` in the changeset file that contains the
StepFinishEvent/toolCalls/toolResults/usage description.
In `@examples/streaming/src/index.ts`:
- Around line 51-52: The logging gate currently only checks (usage.totalTokens
?? 0) > 0 which misses cases where inputTokens or outputTokens are present but
totalTokens is undefined; update the condition around the console.log that uses
usage and stepId to instead log whenever any token field is non-zero, e.g. test
(usage && ((usage.inputTokens ?? 0) > 0 || (usage.outputTokens ?? 0) > 0 ||
(usage.totalTokens ?? 0) > 0)) so inputTokens/outputTokens-only reports are not
skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b607dbc-5218-4d8f-9c83-d26296f5e2ee
📒 Files selected for processing (8)
.changeset/step-finish-ai-sdk-passthrough.mddocs/reference/flow-agent.mdexamples/streaming/src/index.tspackages/agents/ISSUES.mdpackages/agents/src/core/agents/base/agent.test.tspackages/agents/src/core/agents/base/agent.tspackages/agents/src/core/types.tspackages/agents/src/index.ts
💤 Files with no reviewable changes (1)
- packages/agents/ISSUES.md
… StepResult, enrich $.agent() events - Delete `StepInfo` type; inline fields into `StepStartEvent` and `StepFinishEvent` - Add `StepStartEvent` with `stepId`, `stepOperation`, `agentChain` - Redesign `StepFinishEvent`: flat with required `stepId`/`stepOperation`, `output` (was `result`), AI SDK fields via `Partial<AIStepResult>` - Rename flow `StepResult<T>` → `FlowStepResult<T>` (avoids AI SDK collision), `.value` → `.output` - Add `FlowAgentStepResult` — eliminates double-wrapping on `$.agent()` (`result.output` instead of `result.value.output`) - Enrich flow `$.agent()` `onStepFinish` with AI SDK fields (`text`, `toolCalls`, `usage`, `finishReason`, etc.) via `buildFinishEventExtras` - Add `stepOperation: "agent"` to base agent `StepFinishEvent` - Update all tests (658 pass), examples, and documentation Co-Authored-By: Claude <noreply@anthropic.com>
- Fix variable shadowing: rename inner `result` to `generateResult` in agent step - Replace ternary with `match()` for `buildFinishEventExtras` check - Add ts-pattern import to factory.ts - Bump changeset to `major` (breaking API changes) - Improve usage logging gate to check all token fields Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agents/src/integration/lifecycle.test.ts (1)
1689-1723:⚠️ Potential issue | 🟡 MinorThis no longer exercises the globally-unique index path.
stepIdis fixture data, so asserting["a", "b", "c", "d"]only proves the hardcoded IDs are unique. A regression in the sharedindexRef/buildToolCallId()logic would still pass this test. Assert emittedtoolCallIds (or message IDs) instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/integration/lifecycle.test.ts` around lines 1689 - 1723, The test currently only asserts hardcoded step fixture IDs ("a","b","c","d") via the onStepStart stepId—update it to assert runtime-generated global IDs (the shared indexRef / buildToolCallId path) by capturing and asserting the emitted toolCallId or message ID instead; modify the listener in flowAgent (onStepStart) to push the toolCallId/messageId rather than stepId, then change the final expectations to verify uniqueness and execution order against those captured toolCallIds (reference symbols: flowAgent, onStepStart, stepIds array, $outer.step, buildToolCallId, indexRef, toolCallId/messageId).
♻️ Duplicate comments (1)
examples/streaming/src/index.ts (1)
51-52:⚠️ Potential issue | 🟡 MinorGate usage logging on any non-zero token field, not just
totalTokens.The Vercel AI SDK's
LanguageModelUsagetype definesinputTokens,outputTokens, andtotalTokensas independently optional (number | undefined). The current condition(usage.totalTokens ?? 0) > 0will skip the log when a provider omitstotalTokensbut reports input/output counts.Suggested fix
- if (usage && (usage.totalTokens ?? 0) > 0) { + if ( + usage && + ((usage.totalTokens ?? 0) > 0 || + (usage.inputTokens ?? 0) > 0 || + (usage.outputTokens ?? 0) > 0) + ) { console.log(`[step ${stepId}] Tokens: ${usage.inputTokens ?? 0} in / ${usage.outputTokens ?? 0} out`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/streaming/src/index.ts` around lines 51 - 52, The current guard only checks (usage.totalTokens ?? 0) > 0 and will skip logging when a provider omits totalTokens; change the condition to log whenever any token field is present/non-zero by checking usage && ((usage.inputTokens ?? 0) > 0 || (usage.outputTokens ?? 0) > 0 || (usage.totalTokens ?? 0) > 0), and keep the existing console.log that references stepId and usage.inputTokens/outputTokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/flow-agent.md`:
- Around line 269-286: The docs table under onStepFinish incorrectly marks AI
SDK fields as present only on agent tool-loop steps; since the implementation
enriches the flow-level $.agent() onStepFinish payload with the last AI SDK
step, update the StepFinishEvent documentation in docs/reference/flow-agent.md
so the AI SDK fields (stepNumber, text, toolCalls, toolResults, finishReason,
usage, reasoning, sources, response, etc.) are shown as present on Both (i.e.,
available on the outer $.agent() finish event alongside flow fields output and
duration) and ensure the description for onStepFinish/StepFinishEvent mentions
that flow orchestration events are enriched with the last SDK step fields.
In `@packages/agents/docs/core/step.md`:
- Around line 7-15: Examples treat FlowStepResult as the raw value; update each
snippet using $.map, $.each, $.reduce, $.while, and $.all to await the
Promise<FlowStepResult<T>>, branch on the result.ok boolean, and then read the
payload from result.output (or handle the error when ok is false). Locate the
example usages of FlowStepResult and the helper calls ($.map, $.each, $.reduce,
$.while, $.all) and change their destructuring/reads to first await the call,
check `.ok`, and then use `.output` (or propagate/log the StepError) so the
examples match the new FlowStepResult<T> shape.
In `@packages/agents/docs/core/types.md`:
- Around line 161-172: The docs incorrectly describe FlowAgentStepResult as
FlowStepResult<GenerateResult> & { output: string; messages: Message[]; usage:
TokenUsage; finishReason: string }, which makes output both GenerateResult and
string and attaches failure-only fields to the wrong branch; update the
documentation to reflect the actual exported union: the success branch contains
GenerateResult<TOutput> (including the typed output and any GenerateResult
fields) while the failure branch contains the error shape and shared fields
(messages, usage, finishReason) only where appropriate; reference
FlowAgentStepResult, FlowStepResult, GenerateResult (and
GenerateResult<TOutput>), output, messages, usage, finishReason when updating
the doc text so it clearly describes the two-branch union and which fields live
on success vs failure.
In `@packages/agents/src/core/agents/flow/steps/factory.ts`:
- Around line 349-355: The onStepFinish hook currently captures delegated flow
agent finish events as an AIStepResult (lastAIStep.current = event as
AIStepResult) and then lets buildFinishEventExtras() spread all non-discriminant
fields (including output and duration) into the outer $.agent() finish event,
which causes the delegated step's output/duration to overwrite the parent step's
values; fix this by not copying output and duration from delegated flow events
into the parent: change the capture in mergedHooks["onStepFinish"] / lastAIStep
handling to either use a proper type guard instead of blind casting to
AIStepResult and/or ensure buildFinishEventExtras() excludes the keys "output"
and "duration" (or filters them out when merging extras) so only safe extra
fields are propagated.
---
Outside diff comments:
In `@packages/agents/src/integration/lifecycle.test.ts`:
- Around line 1689-1723: The test currently only asserts hardcoded step fixture
IDs ("a","b","c","d") via the onStepStart stepId—update it to assert
runtime-generated global IDs (the shared indexRef / buildToolCallId path) by
capturing and asserting the emitted toolCallId or message ID instead; modify the
listener in flowAgent (onStepStart) to push the toolCallId/messageId rather than
stepId, then change the final expectations to verify uniqueness and execution
order against those captured toolCallIds (reference symbols: flowAgent,
onStepStart, stepIds array, $outer.step, buildToolCallId, indexRef,
toolCallId/messageId).
---
Duplicate comments:
In `@examples/streaming/src/index.ts`:
- Around line 51-52: The current guard only checks (usage.totalTokens ?? 0) > 0
and will skip logging when a provider omits totalTokens; change the condition to
log whenever any token field is present/non-zero by checking usage &&
((usage.inputTokens ?? 0) > 0 || (usage.outputTokens ?? 0) > 0 ||
(usage.totalTokens ?? 0) > 0), and keep the existing console.log that references
stepId and usage.inputTokens/outputTokens.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11427fec-96d4-4287-bb6e-9b28b9708134
📒 Files selected for processing (56)
docs/concepts/flow-agents.mddocs/introduction.mddocs/principles.mddocs/quick-start.mddocs/reference/flow-agent.mdexamples/flow-agent-steps/src/index.tsexamples/flow-agent/src/index.tsexamples/prompts-subagents/src/index.tsexamples/realworld-cli/api/pipeline.tsexamples/streaming/src/index.tspackages/agents/docs/advanced/custom-steps.mdpackages/agents/docs/advanced/streaming.mdpackages/agents/docs/core/flow-agent.mdpackages/agents/docs/core/hooks.mdpackages/agents/docs/core/step.mdpackages/agents/docs/core/types.mdpackages/agents/docs/cost-tracking.mdpackages/agents/docs/create-flow-agent.mdpackages/agents/docs/custom-flow-engine.mdpackages/agents/docs/error-recovery.mdpackages/agents/docs/guides/cost-aware-agents.mdpackages/agents/docs/guides/create-flow-agent.mdpackages/agents/docs/guides/error-recovery.mdpackages/agents/docs/guides/multi-agent-orchestration.mdpackages/agents/docs/guides/test-agents.mdpackages/agents/docs/hooks.mdpackages/agents/docs/multi-agent-orchestration.mdpackages/agents/docs/step-builder.mdpackages/agents/docs/streaming.mdpackages/agents/docs/test-agents.mdpackages/agents/docs/troubleshooting.mdpackages/agents/src/core/agents/base/agent.test.tspackages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/base/utils.tspackages/agents/src/core/agents/flow/engine.test.tspackages/agents/src/core/agents/flow/engine.tspackages/agents/src/core/agents/flow/flow-agent.test.tspackages/agents/src/core/agents/flow/flow-agent.tspackages/agents/src/core/agents/flow/steps/agent.test.tspackages/agents/src/core/agents/flow/steps/all.test.tspackages/agents/src/core/agents/flow/steps/builder.tspackages/agents/src/core/agents/flow/steps/each.test.tspackages/agents/src/core/agents/flow/steps/factory.test-d.tspackages/agents/src/core/agents/flow/steps/factory.test.tspackages/agents/src/core/agents/flow/steps/factory.tspackages/agents/src/core/agents/flow/steps/map.test.tspackages/agents/src/core/agents/flow/steps/race.test.tspackages/agents/src/core/agents/flow/steps/reduce.test.tspackages/agents/src/core/agents/flow/steps/result.tspackages/agents/src/core/agents/flow/steps/step.test.tspackages/agents/src/core/agents/flow/steps/while.test.tspackages/agents/src/core/agents/flow/types.tspackages/agents/src/core/agents/types.tspackages/agents/src/core/types.tspackages/agents/src/index.tspackages/agents/src/integration/lifecycle.test.ts
- Fix StepFinishEvent docs to show AI SDK fields present on flow $.agent() steps - Fix step.md examples to check .ok and use .output for all $ operations - Fix FlowAgentStepResult docs to show correct discriminated union shape - Remove unnecessary AIStepResult casts in factory.ts (use StepFinishEvent directly) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/agents/src/core/agents/flow/steps/factory.ts (1)
422-429:⚠️ Potential issue | 🔴 CriticalDelegated step's
outputanddurationstill overwrite the parent step's fields.The destructure at line 425 excludes only
stepIdandstepOperation, soaiFieldsstill containsoutput,duration, andagentChain. When spread intofinishEventat line 248, these overwrite the parent step's actual values.The parent step's
outputshould be the fullGenerateResult(set at line 245), anddurationshould be the total step time (set at line 246). Only true AI SDK fields (text,toolCalls,toolResults,finishReason,usage,reasoning,sources,response,files,stepNumber, etc.) should be passed through.Proposed fix
buildFinishEventExtras: () => { - // Spread AI SDK fields from the last agent tool-loop step if (isNotNil(lastAIStep.current)) { - const { stepId: _sid, stepOperation: _sop, ...aiFields } = lastAIStep.current as StepFinishEvent; + const { + stepId: _sid, + stepOperation: _sop, + output: _out, + duration: _dur, + agentChain: _chain, + ...aiFields + } = lastAIStep.current as StepFinishEvent; return aiFields; } return {}; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/flow/steps/factory.ts` around lines 422 - 429, The buildFinishEventExtras function is currently spreading lastAIStep.current but only excludes stepId and stepOperation, which lets delegated fields like output, duration, and agentChain overwrite the parent finishEvent; update buildFinishEventExtras (and its destructuring of lastAIStep.current: StepFinishEvent) to explicitly omit output, duration, agentChain (and any other parent-owned fields) and only return true AI SDK fields (text, toolCalls, toolResults, finishReason, usage, reasoning, sources, response, files, stepNumber, etc.); ensure finishEvent assembly (where GenerateResult is set as parent output and duration is computed) remains authoritative and the extras merged do not replace those parent values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/streaming/src/index.ts`:
- Around line 51-57: The logging currently prints only inputTokens/outputTokens
even when usage.totalTokens is provided; update the log in the block that checks
usage to detect when per-type counts are missing and instead include
totalTokens: compute inTokens = usage.inputTokens ?? 0 and outTokens =
usage.outputTokens ?? 0, and if both are 0 but usage.totalTokens is present, log
`[step ${stepId}] Tokens: ${usage.totalTokens} total` (otherwise keep the
existing `in / out` format); adjust the log statement near the usage/stepId
check to use these symbols (usage.totalTokens, usage.inputTokens,
usage.outputTokens, stepId).
---
Duplicate comments:
In `@packages/agents/src/core/agents/flow/steps/factory.ts`:
- Around line 422-429: The buildFinishEventExtras function is currently
spreading lastAIStep.current but only excludes stepId and stepOperation, which
lets delegated fields like output, duration, and agentChain overwrite the parent
finishEvent; update buildFinishEventExtras (and its destructuring of
lastAIStep.current: StepFinishEvent) to explicitly omit output, duration,
agentChain (and any other parent-owned fields) and only return true AI SDK
fields (text, toolCalls, toolResults, finishReason, usage, reasoning, sources,
response, files, stepNumber, etc.); ensure finishEvent assembly (where
GenerateResult is set as parent output and duration is computed) remains
authoritative and the extras merged do not replace those parent values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0e527212-0ab4-45e3-948c-8c3a8410d387
📒 Files selected for processing (3)
.changeset/step-finish-ai-sdk-passthrough.mdexamples/streaming/src/index.tspackages/agents/src/core/agents/flow/steps/factory.ts
When providers report only totalTokens without per-type breakdown, the log now shows the total instead of misleading "0 in / 0 out". Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/agents/docs/core/flow-agent.md`:
- Around line 95-100: Update the `step:finish` stream contract documentation to
include the additional fields surfaced by $.agent() finish events: when
$.agent() returns a StepResult, document that `step:finish` may also contain
`agentChain` and the StepResult payload fields `usage`, `toolCalls`, and
`toolResults` (in addition to `stepId`, `stepOperation`, `output`, and
`duration`), and indicate which fields are optional vs. present only for agent
steps so stream consumers can handle both generic and agent-specific finish
events.
In `@packages/agents/docs/hooks.md`:
- Around line 20-28: Update the flow hook table to match the new public
flow-agent contract: change the `onFinish` event fields from `{ input, result,
duration }` to `{ input, output, duration }` and expand `StepFinishEvent` to
reflect the full `StepResult` shape exposed by `$.agent()` steps (include
`output`, `usage`, `toolCalls`, `toolResults`, `duration`, etc.), and note that
`onStepFinish` fires on both success and error with `output` undefined on error
and `stepId` always present; adjust the textual note beneath the table to
reference `output` and the additional `StepResult` fields so consumers
destructure the correct properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ee457f11-e00f-492b-b196-fd34efcf4bc8
📒 Files selected for processing (14)
docs/reference/flow-agent.mdexamples/streaming/src/index.tspackages/agents/docs/core/flow-agent.mdpackages/agents/docs/core/hooks.mdpackages/agents/docs/core/step.mdpackages/agents/docs/core/types.mdpackages/agents/docs/create-flow-agent.mdpackages/agents/docs/hooks.mdpackages/agents/docs/step-builder.mdpackages/agents/src/core/agents/base/agent.test.tspackages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/flow/steps/factory.tspackages/agents/src/core/types.tspackages/agents/src/index.ts
…bles Update hook and stream event tables to reference StepFinishEvent by name instead of listing a subset of fields, and note that $.agent() steps include all AI SDK StepResult fields (usage, toolCalls, toolResults, etc.). Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
3 tasks
This was referenced Mar 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
StepFinishEventis now a superset of the Vercel AI SDK'sStepResult<ToolSet>— all SDK fields (text,toolCalls,toolResults,finishReason,usage,reasoning,sources,response, etc.) are passed through unchanged, plus funkai additions (stepId,agentChain)onStepFinishstripped tool calls/results to summary fields (argsTextLength,resultTextLength) and simplified usage — now the full AI SDK objects are preservedextractUsage,safeSerializedLength, andextractPropertyhelpers that performed the strippingAIStepResulttype alias for consumersISSUES.md(all 23 issues were closed)Breaking:
toolCallsentries are now full AI SDKTypedToolCallobjects (withinputfield) instead of{ toolName, argsTextLength }.toolResultsare nowTypedToolResultobjects (withoutputfield) instead of{ toolName, resultTextLength }.usageis nowLanguageModelUsage(fields arenumber | undefined) instead of a simplified object with guaranteednumberfields.Test plan