-
Notifications
You must be signed in to change notification settings - Fork 50
fix: Correctly set Append flag in TaskArtifactUpdateEvent and add UpdateArtifactAsync usage examples #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Correctly set Append flag in TaskArtifactUpdateEvent and add UpdateArtifactAsync usage examples #120
Changes from 7 commits
5825f52
854ece7
3848b40
21ec564
b2c3cfb
9ad8d5b
bf9ba4e
8a8af29
083d4c0
17c6c34
8fa7815
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,21 @@ public interface ITaskManager | |
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task ReturnArtifactAsync(string taskId, Artifact artifact, CancellationToken cancellationToken = default); | ||
|
|
||
| /// <summary> | ||
| /// Updates an artifact for a task, either by adding a new artifact or appending to the last one. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// When append is true, the artifact's parts are added to the last artifact in the task's collection. | ||
| /// When append is false or there are no existing artifacts, a new artifact is added to the collection. | ||
| /// </remarks> | ||
| /// <param name="taskId">The ID of the task to update the artifact for.</param> | ||
| /// <param name="artifact">The artifact containing parts to add.</param> | ||
| /// <param name="append">Whether to append to the last artifact (true) or create a new one (false).</param> | ||
| /// <param name="lastChunk">Whether this is the last chunk of the artifact.</param> | ||
| /// <param name="cancellationToken">A cancellation token that can be used to cancel the operation.</param> | ||
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task UpdateArtifactAsync(string taskId, Artifact artifact, bool append = false, bool? lastChunk = null, CancellationToken cancellationToken = default); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot This method is not used yet (except in tests). Show me how it is supposed to be used on agent implementations in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhanced the
The key difference from
This demonstrates the proper usage patterns alongside the existing |
||
|
|
||
| /// <summary> | ||
| /// Updates the status of a task and optionally adds a message to its history. | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -457,6 +457,97 @@ public async Task ReturnArtifactAsync(string taskId, Artifact artifact, Cancella | |
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| throw; | ||
| } | ||
| } | ||
| // TODO: Implement UpdateArtifact method | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public async Task UpdateArtifactAsync(string taskId, Artifact artifact, bool append = false, bool? lastChunk = null, CancellationToken cancellationToken = default) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| if (string.IsNullOrEmpty(taskId)) | ||
| { | ||
| throw new A2AException(nameof(taskId), A2AErrorCode.InvalidParams); | ||
| } | ||
| else if (artifact is null) | ||
| { | ||
| throw new A2AException(nameof(artifact), A2AErrorCode.InvalidParams); | ||
| } | ||
|
|
||
| using var activity = ActivitySource.StartActivity("UpdateArtifact", ActivityKind.Server); | ||
| activity?.SetTag("task.id", taskId); | ||
| activity?.SetTag("artifact.append", append); | ||
| activity?.SetTag("artifact.lastChunk", lastChunk); | ||
|
|
||
| try | ||
| { | ||
| var task = await _taskStore.GetTaskAsync(taskId, cancellationToken).ConfigureAwait(false); | ||
| if (task != null) | ||
| { | ||
| activity?.SetTag("task.found", true); | ||
|
|
||
| task.Artifacts ??= []; | ||
|
|
||
| if (append && task.Artifacts.Count > 0) | ||
| { | ||
| // Append to the last artifact by adding parts to it | ||
| var lastArtifact = task.Artifacts[^1]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello @brandonh-msft. Why are you taking the last artifact from the task ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Matching by The The fix should be: var existing = task.Artifacts?.FirstOrDefault(a => a.ArtifactId == artifact.ArtifactId);
if (existing is not null)
{
existing.Parts.AddRange(artifact.Parts);
}
else
{
task.Artifacts ??= [];
task.Artifacts.Add(artifact);
}This way each artifact chunk is correctly routed to the artifact it belongs to, regardless of insertion order. The |
||
|
|
||
| // Add all parts from the new artifact to the last artifact | ||
| foreach (var part in artifact.Parts) | ||
| { | ||
| lastArtifact.Parts.Add(part); | ||
| } | ||
|
|
||
| activity?.SetTag("event.type", "artifact_append"); | ||
| await _taskStore.SetTaskAsync(task, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // Notify with the updated artifact and append=true | ||
| _taskUpdateEventEnumerators.TryGetValue(task.Id, out var enumerator); | ||
| if (enumerator is not null) | ||
| { | ||
| var taskUpdateEvent = new TaskArtifactUpdateEvent | ||
| { | ||
| TaskId = task.Id, | ||
| Artifact = lastArtifact, | ||
| Append = true, | ||
| LastChunk = lastChunk | ||
| }; | ||
| enumerator.NotifyEvent(taskUpdateEvent); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // Create a new artifact (either append=false or no existing artifacts) | ||
| task.Artifacts.Add(artifact); | ||
| activity?.SetTag("event.type", "artifact_new"); | ||
| await _taskStore.SetTaskAsync(task, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // Notify with the new artifact and append=false | ||
| _taskUpdateEventEnumerators.TryGetValue(task.Id, out var enumerator); | ||
| if (enumerator is not null) | ||
| { | ||
| var taskUpdateEvent = new TaskArtifactUpdateEvent | ||
| { | ||
| TaskId = task.Id, | ||
| Artifact = artifact, | ||
| Append = false, // Always false when creating a new artifact | ||
| LastChunk = lastChunk | ||
| }; | ||
| enumerator.NotifyEvent(taskUpdateEvent); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| activity?.SetTag("task.found", false); | ||
| activity?.SetStatus(ActivityStatusCode.Error, "Task not found"); | ||
| throw new A2AException("Task not found.", A2AErrorCode.TaskNotFound); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| throw; | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.