feat(core): add isFinalStep flag to onStepFinish hook [Closes #1368]#1370
feat(core): add isFinalStep flag to onStepFinish hook [Closes #1368]#1370nneelabh02 wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: a6437b5 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 |
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds an ChangesisFinalStep Hook Flag
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant AISDK
participant createStepHandler
participant onStepFinish
Agent->>AISDK: generateText / streamText with stopWhen
AISDK->>createStepHandler: onStepFinish(event)
createStepHandler->>createStepHandler: await stopWhen(event)
createStepHandler->>onStepFinish: hook args with isFinalStep
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 7454-7470: The `isFinalStep` logic in `agent.ts` is still inferred
from `hasToolCalls` and `maxSteps`, so `hooks.onStepFinish` can receive
`isFinalStep: false` even when a custom `stopWhen` has already resolved the
loop. Update the step-finalization logic around the `event`,
`conversationSteps`, and `hooks.onStepFinish` flow to use the actual resolved
stop condition from the loop decision, and pass that result into `isFinalStep`
instead of deriving it only from tool calls or step count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e077160-5927-4e7d-9099-0ffd1a2aacfa
📒 Files selected for processing (3)
packages/core/src/agent/agent.spec.tspackages/core/src/agent/agent.tspackages/core/src/agent/hooks/index.ts
b136b6f to
7b3216d
Compare
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/core/src/agent/agent.ts (1)
7458-7469: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
stopWhenis invoked with the wrong argument shape.AI SDK's
StopConditionpredicate signature expects{ steps: StepResult[] }, but the code at line 7462 callsawait stopWhen(event)passing only the current singleStepResult. Built-in conditions likestepCountIs(n)andhasToolCall(...)destructure the fullstepsarray to make decisions—they cannot work correctly without it. Custom stop conditions that inspect step history will also fail.The codebase already accumulates steps in
oc.systemContext.get("conversationSteps"); this array (plus the current step) should be passed as the required{ steps: [...] }object.🐛 Proposed fix
- const isStopped = await stopWhen(event); + const priorSteps = (oc.systemContext.get("conversationSteps") as StepResult<ToolSet>[]) ?? []; + const isStopped = await stopWhen({ steps: [...priorSteps, event] });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/agent/agent.ts` around lines 7458 - 7469, The stop condition call in agent.ts is passing the current StepResult directly, but stopWhen expects the AI SDK StopCondition shape with a steps array. Update the final-step logic around the stopWhen invocation to build and pass { steps: [...] } using the accumulated conversationSteps from oc.systemContext plus the current event, so built-in conditions like stepCountIs and hasToolCall and any custom history-based predicates can evaluate correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 7458-7469: The stop condition call in agent.ts is passing the
current StepResult directly, but stopWhen expects the AI SDK StopCondition shape
with a steps array. Update the final-step logic around the stopWhen invocation
to build and pass { steps: [...] } using the accumulated conversationSteps from
oc.systemContext plus the current event, so built-in conditions like stepCountIs
and hasToolCall and any custom history-based predicates can evaluate correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64a541e4-062e-43d7-b0ab-b71f53fc6b37
📒 Files selected for processing (2)
packages/core/src/agent/agent.spec.tspackages/core/src/agent/agent.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/agent/agent.spec.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@Diluka please check this PR, thank you :) |
2 participants
This PR implements the
isFinalSteplogic within the core execution loop foronStepFinish, as discussed in #1368.It correctly determines if a step is the final one by calculating window sizes on a per-step basis, avoiding the previous approach of waiting for
onEnd. This allows the user to accurately know if the current step is the final one, preventing mid-task context overflows.Justification for modifying
onStepFinishinstead ofonEnd:The original issue (#1368) requested exposing
lastStepUsagein theonEndhook. However, as discussed with the maintainers, window size calculations must happen per-step to effectively prevent mid-task context overflows. Waiting untilonEndis too late. Therefore, we decided to leaveonEnduntouched and instead add anisFinalStepflag to theonStepFinishhook. This fully addresses the underlying use case of #1368 while adopting a more robust architecture.Resolves #1368
Summary by CodeRabbit
New Features
Bug Fixes