feat(agents): input parameter parity with Vercel AI SDK#65
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: 578b809 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.
|
|
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:
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)
📝 WalkthroughWalkthroughReplaces ad-hoc StepFinishEvent assembly with explicit AI-backed types and factory functions: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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>
|
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>
Replace `Partial<AIStepResult>` with full `AIStepResult` intersection so all AI SDK fields are always present. Flow steps get sensible defaults via EMPTY_AI_STEP constant. Removes redundant toolCalls and toolResults re-declarations. Renames factory functions: - stepFinishEventFromAIStep → createAgentStepFinishEvent - stepFinishEventFromFlow → createFlowStepFinishEvent Co-Authored-By: Claude <noreply@anthropic.com>
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)