feat(chat): hand run() a streamText with the managed options already applied - #1
Conversation
…applied Source PR: triggerdotdev#4884 Source head: bf66457
|
| }, | ||
|
|
||
| run: async ({ messages, signal }) => { | ||
| run: async ({ messages, signal, streamText }) => { |
There was a problem hiding this comment.
Shipwright · HIGH
The docs repeatedly show 'run: async ({ messages, signal, streamText }) => streamText({...})' without explaining that 'streamText' here shadows the 'ai' import.
Impact: The docs repeatedly show 'run: async ({ messages, signal, streamText }) => streamText({...})' without explaining that 'streamText' here shadows the 'ai' import. A reader who imports 'streamText' from 'ai' and also destructures it will be confused about which one is managed; the note exists in backend.mdx but is easy to miss in the other examples.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| "@trigger.dev/sdk": patch | ||
| --- | ||
|
|
||
| Undo, edit and regenerate now survive a run ending. History rolled back from `onAction` was only kept in the running worker's memory, so the rollback held while that worker stayed warm and then reverted on the next continuation. The undone messages came back, minutes later, with no error. This also holds when the turn before the action failed: the rollback used to be written against the cursor from before that turn, so a continuation could replay output the failed turn had already superseded. |
There was a problem hiding this comment.
Shipwright · HIGH
The changeset for 'persist-action-history-mutations' claims rollback persistence now survives a run ending, but the diff contains no implementation code for this fix — only the cha
Impact: The changeset for 'persist-action-history-mutations' claims rollback persistence now survives a run ending, but the diff contains no implementation code for this fix — only the changeset and tests. If the runtime change is in a separate package not shown in this diff, the release is incomplete; if it is supposed to be here, the fix is missing entirely.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| run: async ({ messages, signal, streamText }) => | ||
| streamText({ model, messages, abortSignal: signal }); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Shipwright · HIGH
The managed 'streamText' throws when 'system' is set in two places, but the diff only documents this in changesets and docs.
Impact: The managed 'streamText' throws when 'system' is set in two places, but the diff only documents this in changesets and docs. There is no test in the visible diff that exercises the throw path, so a regression where the throw is skipped or the wrong value wins would ship silently.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| const chunks: LanguageModelV3StreamPart[] = [ | ||
| { type: "text-start", id: "t1" }, | ||
| { type: "text-delta", id: "t1", delta: "partial" }, | ||
| ]; |
There was a problem hiding this comment.
Shipwright · HIGH
The steering tests rely on 'sessionStreams.lastSeqNum' cast through 'as unknown as SeqReader' to observe internal sequence numbers.
Impact: The steering tests rely on 'sessionStreams.lastSeqNum' cast through 'as unknown as SeqReader' to observe internal sequence numbers. This couples tests to an internal API shape; if 'sessionStreams' changes, the tests break without testing the actual behavior.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| "@trigger.dev/sdk": minor | ||
| --- | ||
|
|
||
| Actions are sent through `useChat` so a turn that follows one renders like any turn. `TriggerChatTransport` recognises `body.action` on a `useChat` request and sends it as an action, so `sendMessage(undefined, { body: { action } })` or `regenerate({ body: { action } })` sends the action and `useChat` owns the response: it streams into the message list, `status` and `error` behave as for a message, and `stop` works. `useChatActions({ sendMessage })` in `@trigger.dev/sdk/chat/react` is a two-line convenience over that. |
There was a problem hiding this comment.
Shipwright · HIGH
The 'useChatActions' convenience sends arbitrary 'action' objects through 'useChat' request bodies.
Impact: The 'useChatActions' convenience sends arbitrary 'action' objects through 'useChat' request bodies. The changeset says the backend validates against 'actionSchema', but the diff does not show the validation path for the new 'body.action' transport route. If validation is bypassed or the schema is permissive, a client could inject action types the server did not intend to expose.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| "@trigger.dev/sdk": patch | ||
| --- | ||
|
|
||
| Injected system context is merged into a single instruction block, so it works on every supported AI SDK version. Note that a cached system prompt gives up its cache entry for as long as an injection is live, since the cached prefix has changed. |
There was a problem hiding this comment.
Shipwright · LOW
The changeset for 'inject-instructions-shape' notes that a cached system prompt gives up its cache entry while an injection is live, but does not quantify the cost or suggest a mit
Impact: The changeset for 'inject-instructions-shape' notes that a cached system prompt gives up its cache entry while an injection is live, but does not quantify the cost or suggest a mitigation. A maintainer tuning prompt caching later will not know whether this is a minor or major performance regression.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
Summary
Every
run()had to spreadchat.toStreamTextOptions(), and leaving it out dropped six things with no error: the managed prompt and its cache control, the registry-resolved model, the prompt's sampling config, telemetry, the skill tools, and theprepareStepthat delivers steering, compaction and injected context.Before:
After:
streamTextcomes fromrun's argument and shadows the one imported fromai, so the correct call is now the shorter one and the managed options cannot be lost by omission.chat.toStreamTextOptions()is unchanged and still supported, and is still the only option in a custom agent.What changes when your options collide with the managed ones
Spread order decides the outcome today, and losing is silent:
The managed
streamTextmerges instead.toolsare passed into the helper so skill tools survive, and aprepareStepyou pass runs after the managed one rather than replacing it. Everything else you name is left alone and wins, telemetry included.systemis the exception: it can be set onchat.agent({ system }), throughchat.prompt.set(), or at the call site, but only in one of them. Two at once throws and names the one that already owns it. No shape merges two system values across every supported AI SDK version, since v5 rejects an array of blocks and a structured block carries the provider options that make prompt caching work.chat.headStart and chat.startHeadStart
buildStreamTextOptionssuppliesmessages,stopWhen: stepCountIs(1)andabortSignal. Step 1 belongs to the route handler and step 2 onward to the agent, so re-settingstopWhenafter a spread hands over a stream that has already run past step 1.Before:
After:
Passing
messages,prompt,stopWhenorabortSignalto thatstreamTextis a type error, with a runtime throw behind it for JavaScript callers.toolsis yours to pass. The old shape only warned in prose.Also in here
chat.agent()takessystem,registry,cacheControlandsystemProviderOptions, so a managed prompt's model and its cache breakpoint no longer have to be passed at the call site.ChatStreamTextis exported for typing a loop factored out ofrun.The signature is taken from the AI SDK's own declaration:
The peer range spans
aiv5, v6 and v7, whose options differ.typeofresolves to whichever version is installed, so generics and tool inference are the caller's own and a v8 option needs no change here.Actions.
onActionno longer receivesstreamTextortools: an action is a state edit, and one that returnschat.turn()(added in triggerdotdev#4816) is followed byrun(), which already has both. The action docs on this branch describe that model.chat.toStreamTextOptions()now also applieschat.agent'ssystem,registry,cacheControlandsystemProviderOptions, so the spread form is equivalent to thestreamTexthanded torun(), as the docs say; previously an agent's system prompt was silently dropped on that path. Those options are published on every boot, including for ahydrateMessagesagent, which skips the snapshot boot block where they were first set.Verification
Typecheck and the full suite pass on both
ai@6.0.116andai@7.0.66. The option merge is a pure function so the merged object can be asserted directly, which is howexperimental_telemetrybeing dropped was caught: moststreamTextoptions never reach the provider, so a test that observes the model cannot see them.Run end to end against a deployed agent with every
runrewritten to the new form and no spread anywhere: steering, undo across a cold boot, and regenerate all still pass, a caller's ownprepareStepruns while managed steering still fires inside the turn, and consecutive injections arrive one per turn. The handover-owned options are pinned by@ts-expect-errorassertions in a typechecked test rather than only by the runtime throw.Source merge-base:
f8aacacb8fa05d5044aa3853dce829eb71f61c48Source head:
bf66457dfc4e8e9fbd6f06a1ee7b2cc6a0877395