feat(agents): input parameter parity with Vercel AI SDK#65
Open
zrosenbauer wants to merge 6 commits intomainfrom
Open
feat(agents): input parameter parity with Vercel AI SDK#65zrosenbauer wants to merge 6 commits intomainfrom
zrosenbauer wants to merge 6 commits intomainfrom
Conversation
Surface CallSettings (temperature, maxOutputTokens, topP, topK, presencePenalty, frequencyPenalty, stopSequences, seed, maxRetries), telemetry (experimental_telemetry), granular timeout objects, custom stop conditions (stopWhen), and stream-only callbacks (onChunk, onStreamError, onAbort) on both AgentConfig and per-call overrides. Closes #62 Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 1623a5d 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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR refactors Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
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/core/agents/flow/flow-agent.ts (1)
296-315:⚠️ Potential issue | 🟡 MinorFix nested ternary to comply with no-ternary rule.
Lines 301-303 use a nested ternary expression, which violates the project's
no-ternarylint rule (flagged by static analysis). Useif/elseblocks instead.Proposed fix using if/else
function resolveSignal(params: FlowGenerateParams): AbortSignal { const { timeout, signal } = params; - // Object timeout — use totalMs for the flow-level signal (stepMs/toolMs - // are forwarded to individual agent calls, not handled here) - const timeoutMs = typeof timeout === "number" - ? timeout - : (typeof timeout === "object" && isNotNil(timeout) ? timeout.totalMs : undefined); + // Object timeout — use totalMs for the flow-level signal (stepMs/toolMs + // are forwarded to individual agent calls, not handled here). + let timeoutMs: number | undefined; + if (typeof timeout === "number") { + timeoutMs = timeout; + } else if (typeof timeout === "object" && isNotNil(timeout)) { + timeoutMs = timeout.totalMs; + } if (signal && isNotNil(timeoutMs)) { return AbortSignal.any([signal, AbortSignal.timeout(timeoutMs)]); } if (isNotNil(timeoutMs)) { return AbortSignal.timeout(timeoutMs); } if (signal) { return signal; } return new AbortController().signal; }As per coding guidelines: "No ternaries — use
match(),if/else, or early returns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/flow/flow-agent.ts` around lines 296 - 315, The nested ternary in resolveSignal that computes timeoutMs from FlowGenerateParams (using timeout and isNotNil) violates the no-ternary rule; replace it with a clear if/else block: declare let timeoutMs: number | undefined, then if typeof timeout === "number" set timeoutMs = timeout, else if typeof timeout === "object" && isNotNil(timeout) set timeoutMs = timeout.totalMs, else leave undefined; keep the rest of resolveSignal logic that uses signal, AbortSignal.timeout and AbortSignal.any unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/agents/src/core/agents/flow/flow-agent.ts`:
- Around line 296-315: The nested ternary in resolveSignal that computes
timeoutMs from FlowGenerateParams (using timeout and isNotNil) violates the
no-ternary rule; replace it with a clear if/else block: declare let timeoutMs:
number | undefined, then if typeof timeout === "number" set timeoutMs = timeout,
else if typeof timeout === "object" && isNotNil(timeout) set timeoutMs =
timeout.totalMs, else leave undefined; keep the rest of resolveSignal logic that
uses signal, AbortSignal.timeout and AbortSignal.any unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: abd7dcf4-a9f7-4a71-a344-077826049389
📒 Files selected for processing (5)
.changeset/input-param-parity.mdpackages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/flow/flow-agent.tspackages/agents/src/core/agents/types.tspackages/agents/src/index.ts
Merge duplicate ai import, replace nested ternary with if/else in flow-agent resolveSignal, capitalize comment. 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/src/core/agents/base/agent.ts`:
- Around line 814-821: Change the stopWhen types to accept either a single
StopCondition or an array and update buildStopConditions to accept and normalize
both forms: update GenerateParams.stopWhen and AgentGenerateOverrides.stopWhen
to the widened type StopCondition<ToolSet> | StopCondition<ToolSet>[] |
undefined, and change buildStopConditions(maxSteps, userConditions) to accept
userConditions: StopCondition<ToolSet> | StopCondition<ToolSet>[] | undefined;
implement logic so if userConditions is nil return stepCountIs(maxSteps), if
it's an array return [...userConditions, stepCountIs(maxSteps)], and if it's a
single StopCondition return [userConditions, stepCountIs(maxSteps)] so the
stepCountIs is always enforced.
In `@packages/agents/src/core/agents/flow/flow-agent.ts`:
- Around line 299-311: The resolveSignal logic in flow-agent.ts only reads
timeout.totalMs and silently drops stepMs/chunkMs, so update resolveSignal (the
function that extracts timeoutMs) to either narrow the accepted timeout type to
number | { totalMs?: number } or validate the passed object and throw when
unsupported keys (stepMs, chunkMs, etc.) are present; ensure callers like
flowAgent.generate and flowAgent.stream cannot pass nested timeout objects with
stepMs/chunkMs that will be ignored by: 1) adjusting the TypeScript type for the
timeout parameter to remove stepMs/chunkMs, or 2) adding a runtime check inside
resolveSignal that inspects the timeout object and throws a clear error if any
unknown keys (e.g., "stepMs", "chunkMs") are found, otherwise continue to use
totalMs to build the AbortSignal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b36fdb41-2291-40c6-a68b-44596649b252
📒 Files selected for processing (3)
packages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/flow/flow-agent.tspackages/agents/src/core/agents/types.ts
Address PR review feedback: the AI SDK accepts `StopCondition | StopCondition[]` for `stopWhen`. Widen the type on AgentGenerateOverrides and update buildStopConditions() to normalize both forms. Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
Re: flow agent timeout object handling — created #67 to track this as a follow-up. The flow agent currently only supports |
Make toolCalls and toolResults required fields on StepFinishEvent instead of hiding them behind Partial<AIStepResult>. Add factory functions (stepFinishEventFromAIStep, stepFinishEventFromFlow) so event construction is centralized and flow steps stub empty arrays. Add type tests ensuring StepFinishEvent.toolCalls and toolResults match the AI SDK's StepResult types. Closes #66 Co-Authored-By: Claude <noreply@anthropic.com>
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
Closes #62
AgentConfigand per-call overrides:temperature,maxOutputTokens,topP,topK,presencePenalty,frequencyPenalty,stopSequences,seed,maxRetriestimeoutnow acceptsnumber | { totalMs?, stepMs?, toolMs?, ... }forwarded to the AI SDKexperimental_telemetry) on both config and per-call overridesstopWhen) — combined with internalstepCountIs(maxSteps)safety ceilingonChunk,onStreamError(AI SDK'sonError),onAbortStopConditionandTelemetrySettingsfrom the public APIAll new fields are optional — no breaking changes.
Test plan
pnpm typecheck— all 22 tasks passpnpm build --filter=@funkai/agents— clean buildpnpm test --filter=@funkai/agents— 644/644 tests pass, no type errorstemperature,maxOutputTokens)stopWhenwith custom stop conditionsonChunk,onStreamError,onAbort)