feat: add onStepFinish callback to text generation functions#18
feat: add onStepFinish callback to text generation functions#18
Conversation
- Introduced an optional onStepFinish parameter to generateTextWithModel, handleTextSmall, and handleTextLarge functions. - Enhanced the text generation process by allowing users to provide a callback that executes after each step, enabling custom handling of step results.
WalkthroughAdded an optional Changes
Sequence DiagramsequenceDiagram
participant User
participant generateText as generateTextWithModel
participant handler as handleTextSmall / handleTextLarge
participant toolLogic as Tool Handling
participant callback as onStepFinish
User->>generateText: Call with params + onStepFinish
generateText->>handler: Forward params (includes onStepFinish)
handler->>toolLogic: Run tool/internal step
toolLogic-->>handler: Return stepResult (captured)
alt onStepFinish provided
handler->>callback: Invoke with stepResult
callback-->>handler: Return (sync or awaited)
end
handler-->>generateText: Return final string / ToolResponse
generateText-->>User: Return result
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/models/text.ts (4)
21-21: Improve type safety forstepResultparameter.The
stepResult: anytype reduces type safety. Consider using a proper type from the AI SDK if available, or defining an interface that describes the expected structure of the step result.- onStepFinish?: (stepResult: any) => void | Promise<void>; + onStepFinish?: (stepResult: StepResult) => void | Promise<void>;You may need to import or define
StepResultbased on the AI SDK's type definitions.
74-103: Document the constraint that callback only works with tools.The
onStepFinishcallback is only invoked when tools are provided (due to theif (tools)check on line 74). If a user providesonStepFinishwithout providing tools, the callback will never execute, which could be confusing. Consider adding validation or documentation to clarify this constraint.Add a warning if
onStepFinishis provided without tools:+ // Warn if onStepFinish is provided without tools + if (onStepFinish && !tools) { + logger.warn('[OpenRouter] onStepFinish callback provided but will not be invoked because no tools were specified'); + } + if (tools) {
142-151: Add JSDoc documentation for the new parameter.The
onStepFinishparameter lacks documentation explaining its purpose, when it's invoked, and the structure of thestepResultparameter. Adding JSDoc would improve developer experience./** * TEXT_SMALL model handler + * @param runtime - The agent runtime + * @param params - Generation parameters + * @param params.onStepFinish - Optional callback invoked after each generation step (only when tools are used) */ export async function handleTextSmall(
156-165: Add JSDoc documentation for the new parameter.Similarly, add documentation for the
onStepFinishparameter inhandleTextLargeto maintain consistency and improve developer experience./** * TEXT_LARGE model handler + * @param runtime - The agent runtime + * @param params - Generation parameters + * @param params.onStepFinish - Optional callback invoked after each generation step (only when tools are used) */ export async function handleTextLarge(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/models/text.ts(5 hunks)
🔇 Additional comments (1)
src/models/text.ts (1)
24-24: LGTM!Destructuring the
onStepFinishparameter is correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/text.ts (1)
74-106: Fix:onStepFinishcallback is only executed when tools are provided, violating principle of least surprise.The function accepts
onStepFinishas an optional parameter (line 21) but only invokes it whentoolsare provided (line 74). When tools are absent, the callback is silently ignored despite being passed by the user.This creates an inconsistent API where a parameter is accepted but conditionally disregarded without documentation or type-system enforcement. Either:
- Extend the callback to work for all generation types, or
- Document this limitation in the function's JSDoc and potentially enforce it at the type level (e.g.,
onStepFinishis only valid whentoolsare defined)
🧹 Nitpick comments (1)
src/models/text.ts (1)
21-21: Consider improving type safety for the stepResult parameter.The
stepResultparameter is typed asany, which reduces type safety. Consider using a more specific type from the AI SDK (e.g., from theaipackage's types) to provide better IntelliSense and catch potential errors at compile time.For example:
- onStepFinish?: (stepResult: any) => void | Promise<void>; + onStepFinish?: (stepResult: StepResult) => void | Promise<void>;You may need to import the appropriate type from the
aipackage if it's exported.Also applies to: 151-151, 165-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/models/text.ts(5 hunks)
🔇 Additional comments (1)
src/models/text.ts (1)
98-105: Good implementation of error handling for user callback.The error handling correctly addresses the previous review feedback. The try-catch block ensures that errors in the user-provided callback won't break the text generation flow, and the error logging properly handles both Error instances and other thrown values.
2 participants
Summary by CodeRabbit