.NET: feat: adds the executorId as an additional property when running the workflow as agent in dotnet - #7934
Vincent Biret (baywet) wants to merge 13 commits into
Conversation
…workflow as agent in dotnet
There was a problem hiding this comment.
Pull request overview
Adds workflow executor attribution to streamed .NET agent response updates.
Changes:
- Adds a public
executorIdmetadata key. - Attaches executor IDs to response updates.
- Adds multi-executor streaming coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
WorkflowAgentAdditionalProperties.cs |
Defines the executor ID key. |
AgentResponseUpdateEvent.cs |
Adds attribution metadata to updates. |
WorkflowHostSmokeTests.cs |
Tests attribution and metadata preservation. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…cross execution methods Signed-off-by: Vincent Biret <vibiret@microsoft.com>
…revious inputs for the executorId Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
| Throw.IfNull(message); | ||
|
|
||
| return new(message.Role, message.Contents) | ||
| return SetExecutorId(new(message.Role, message.Contents) |
There was a problem hiding this comment.
The behaviour pre-existed this change, Tao Chen (@TaoChenOSU) to confirm whether we should fix that or not
| MessageId = Guid.NewGuid().ToString("N"), | ||
| Role = ChatRole.Assistant, | ||
| ResponseId = this.LastResponseId, | ||
| RawRepresentation = evt |
There was a problem hiding this comment.
Maybe I'm wrong, but I would have thought that the raw representation would typically be some sort of WorkflowEvent, and if so, that users could get the executor id from that.
There was a problem hiding this comment.
I don't think I have enough context to reply to this comment. Why should it be a WorkflowEvent as opposed to an Executor Event? What parameters are you taking into consideration for this design suggestion?
There was a problem hiding this comment.
I don't really know enough about this area either to know all the different raw representation types that might be represented here, but I think it's a good thing to verify before making the change.
Peter Ibekwe (@peibekwe), might have a better sense.
Co-authored-by: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com>
| /// <summary> | ||
| /// Defines additional property keys used by workflow-hosted agents. | ||
| /// </summary> | ||
| public static class WorkflowAgentAdditionalProperties |
There was a problem hiding this comment.
FYI: If we are unsure about something new that is public we often mark it as experimental, to make sure we can still make changes to it before graduating the feature. We then remove experimental once we are happy with the overall change.
Not sure if that is applicable here, but just an FYI.
There was a problem hiding this comment.
Thanks I think Tao Chen (@TaoChenOSU) and Peter Ibekwe (@peibekwe) will be able to inform the design here and whether or not we need to add those experimental aspects.
|
worth exploring this instead of magic keys in a dictionary update.RawRepresentation is ExecutorEvent exevt
exevt.ExecutorId |
fixes #4445