fix: persist modelId in assistant message content.metadata#12969
Conversation
|
@catdalfonso is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 937c211 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
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:
WalkthroughPersist the LLM response model identifier by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🤖 Fix all issues with AI agents
In `@packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts`:
- Around line 1012-1022: The code currently stamps responseModelId from
runState.state.responseMetadata onto every assistant message returned by
messageList.get.response.db(), which wrongly overwrites prior iterations'
modelIds; change the loop that iterates over messageList.get.response.db() (and
the code that sets msg.content.metadata) to only update messages whose messageId
matches the current step's response messages (use the current iteration's
message IDs kept on the run state or the subset returned by messageList for this
step) so only assistant messages produced in the current LLM execution (not
earlier ones) receive the responseModelId.
🧹 Nitpick comments (1)
.changeset/olive-bees-film.md (1)
5-5: Remove internal implementation detail from changeset description.The phrase "from response-metadata stream chunks" exposes an internal implementation detail. Per the changeset guidelines, focus on the outcome for downstream consumers rather than how the data is sourced internally.
Consider simplifying to something like:
Fixed
modelIdnot being persisted in assistant messagecontent.metadata. Downstream consumers (storage adapters, processors) can now read which model generated each assistant message viacontent.metadata.modelId.As per coding guidelines, changeset descriptions should "Highlight outcomes! What does change for the end user? Do not focus on internal implementation details" and use action-oriented verbs like "Fixed" for bug fixes.
Currently llm-execution-step.ts captures modelId in runState.state.responseMetadata during streaming but never propagates it to MastraDBMessage.content.metadata before messages are saved. This means downstream consumers (storage adapters, processors) that read content.metadata.modelId receive undefined. Inject responseMetadata.modelId into content.metadata of all assistant response messages after they are added to the MessageList and before output processors run. Co-authored-by: Cursor <cursoragent@cursor.com>
catdalfonso
force-pushed
the
feature/persist-model-id-in-message-metadata
branch
from
484a657 to
1957c4a
Compare
| if (responseModelId) { | ||
| for (const msg of messageList.get.response.db()) { | ||
| if (msg.id === messageId && msg.role === 'assistant') { | ||
| msg.content.metadata = { ...msg.content.metadata, modelId: responseModelId }; |
There was a problem hiding this comment.
hmm I'm not sure this is the right way to do it, I think we should add it when we add messages to message list during streaming. The problem here is om "seals" messages now, breaking up the same agents response message into smaller chunks (each with unique ids) so that background buffering/retrieval of parts of messages works properly
There was a problem hiding this comment.
Good call, I moved this around to better support OM.
…re adding to message list
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/olive-bees-film.md:
- Line 5: Rewrite the changeset entry for the addition of modelId so it uses
short, direct, outcome-focused sentences aimed at developers: state what changed
(assistant messages now include a modelId in content.metadata), what users or
downstream systems will see (messages carry the model identifier so storage
adapters and processors can determine the model that generated a message), and
avoid internal implementation details (omit mentions of streaming internals,
sealed/split observational memory handling). Locate the text referencing
modelId, assistant message, and content.metadata in the current changeset and
replace it with 2–3 concise sentences that emphasize the observable behavior and
impact.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/olive-bees-film.mdpackages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
3 participants
Description
Currently,
llm-execution-step.tscapturesmodelIdinrunState.state.responseMetadataduring streaming but never propagates it toMastraDBMessage.content.metadatabefore messages are saved. This means downstream consumers (storage adapters, processors) that readcontent.metadata.modelIdreceiveundefined.Inject
responseMetadata.modelIdintocontent.metadataof all assistant response messages after they are added to theMessageListand before output processors run.Type of Change
Checklist
Summary by CodeRabbit
New Features
Tests
Chores