Repository navigation
Conversation
|
|
Hi @cocvn157, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed.
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| responseCount = 0; | ||
| } | ||
| const responses = input.steps.at(-1)?.response.messages ?? []; | ||
| const responses = input.responseMessages ?? input.steps.at(-1)?.response.messages ?? []; |
There was a problem hiding this comment.
🟡 Missing release note for SDK fix
This customer-facing retainStepMessages fix has no changeset. The package changelog will omit the repaired multi-step tool behavior.
Learn more
The SDK is a published package, and this change fixes a failure customers can encounter when a conversation uses multiple tool steps. The repository requires a changeset for user-facing package bug fixes. None is included in this PR.
Example: A customer upgrades to the release containing this change to fix missing tool results. The package changelog gives them no notice that the fix is available.
Recommended fix: Add a patch changeset for @trigger.dev/sdk with a one-sentence, user-facing description of the restored tool context.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| }); | ||
|
|
||
| it("accumulates tool results across 3+ steps on AI SDK 7 where step.response.messages is per-step", async () => { |
There was a problem hiding this comment.
🟡 Missing regression comment on new test
The new retainStepMessages test lacks the required // Catches: <bug> comment identifying the regression it protects.
Learn more
Repository testing guidance requires every new unit test to identify its distinct regression in a one-line // Catches: <bug> comment. This test exercises loss of later tool results after a prepare-step message override, but it does not include that comment.
Example: A future maintainer reading this test sees a three-step scenario without the mandatory comment explaining that the regression is dropped tool results after step two.
Recommended fix: Place a // Catches: AI SDK 7 dropping tool results after the second step when prepareStep returns messages. comment directly above this test.
Was this helpful? React with 👍 or 👎 to provide feedback.
| responseCount = 0; | ||
| } | ||
| const responses = input.steps.at(-1)?.response.messages ?? []; | ||
| const responses = input.responseMessages ?? input.steps.at(-1)?.response.messages ?? []; |
There was a problem hiding this comment.
🔍 Compaction rebuild retains the old response source
The separate chatCompact rebuild still slices per-step responses by a cumulative count. Compaction-enabled AI SDK 7 agents can therefore lose later tool results despite this retention fix.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #5077
In AI SDK 7,
step.response.messagescontains only the messages produced in that single step rather than accumulating across all steps in the turn. BecauseretainStepMessagesreadinput.steps.at(-1)?.response.messagesand sliced fromresponseCount, steps 3 and later sliced past the 2 per-step messages and dropped all subsequent tool results wheneverprepareStepreturnedmessages.This prefers
input.responseMessages(which AI SDK 7 populates with the accumulated response messages across all steps so far) before falling back toinput.steps.at(-1)?.response.messagesfor AI SDK 5 and 6.