rysweet-unsubscribe-and-agent-tests-4744 #4920

rysweet · 2025-01-07T17:20:23Z

Why are these changes needed?

adds unsubscribe api
adds tests for most all of Agent and AgentWorker

Related issue number

closes #4744
closes #4357

Checks

I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
I've added tests (if relevant) corresponding to the changes introduced in this PR.
I've made sure all auto checks have passed.

Copilot

Copilot reviewed 105 out of 120 changed files in this pull request and generated 3 comments.

Files not reviewed (15)

dotnet/AutoGen.sln: Language not supported
dotnet/Directory.Packages.props: Language not supported
dotnet/samples/dev-team/DevTeam.AgentHost/DevTeam.AgentHost.csproj: Language not supported
dotnet/samples/dev-team/DevTeam.AgentHost/Properties/launchSettings.json: Language not supported
dotnet/samples/dev-team/DevTeam.AgentHost/appsettings.Development.json: Language not supported
dotnet/samples/dev-team/DevTeam.Agents/DevTeam.Agents.csproj: Language not supported
dotnet/samples/dev-team/DevTeam.Agents/Properties/launchSettings.json: Language not supported
dotnet/samples/dev-team/DevTeam.Agents/appsettings.Development.json: Language not supported
dotnet/samples/dev-team/DevTeam.AgentHost/Program.cs: Evaluated as low risk
dotnet/samples/dev-team/DevTeam.Agents/Developer/Developer.cs: Evaluated as low risk
dotnet/samples/dev-team/DevTeam.Agents/DeveloperLead/DeveloperLead.cs: Evaluated as low risk
dotnet/samples/dev-team/DevTeam.Agents/ProductManager/ProductManager.cs: Evaluated as low risk
dotnet/samples/dev-team/DevTeam.Agents/Program.cs: Evaluated as low risk
README.md: Evaluated as low risk
dotnet/samples/Hello/HelloAIAgents/Program.cs: Evaluated as low risk

Comments suppressed due to low confidence (1)

dotnet/samples/Hello/HelloAgent/Program.cs:59

The SayHello method now accepts a CancellationToken, but the Handle method does not pass the cancellationToken to it. Ensure that the cancellationToken is propagated correctly.

public async Task<string> SayHello(string ask, CancellationToken cancellationToken = default)

Copilot · 2025-01-07T17:21:54Z

dotnet/samples/Hello/HelloAgentState/Program.cs

            typeRegistry),
            IHandleConsole,
            IHandle<NewMessageReceived>,
            IHandle<ConversationClosed>,
            IHandle<Shutdown>
    {
        private AgentState? State { get; set; }
-        public async Task Handle(NewMessageReceived item)
+        public async Task Handle(NewMessageReceived item, CancellationToken cancellationToken = default)


The CancellationToken parameter is not being used within the method to handle cancellation.

Suggested change

public async Task Handle(NewMessageReceived item, CancellationToken cancellationToken = default)

var response = await SayHello(item.Message, cancellationToken).ConfigureAwait(false);

Copilot · 2025-01-07T17:21:54Z

dotnet/samples/Hello/HelloAgentState/Program.cs

@@ -57,7 +55,7 @@ await StoreAsync(new AgentState
            await PublishMessageAsync(new Shutdown { Message = this.AgentId.Key }).ConfigureAwait(false);

        }
-        public async Task Handle(ConversationClosed item)
+        public async Task Handle(ConversationClosed item, CancellationToken cancellationToken = default)


The CancellationToken parameter is not being used within the method to handle cancellation.

Suggested change

public async Task Handle(ConversationClosed item, CancellationToken cancellationToken = default)

State = await ReadAsync<AgentState>(this.AgentId, cancellationToken).ConfigureAwait(false);

Copilot · 2025-01-07T17:21:54Z

dotnet/samples/Hello/HelloAgentState/Program.cs

@@ -74,7 +72,7 @@ await StoreAsync(new AgentState
                TextData = JsonSerializer.Serialize(state)
            }).ConfigureAwait(false);
        }
-        public async Task Handle(Shutdown item)
+        public async Task Handle(Shutdown item, CancellationToken cancellationToken = default)


The CancellationToken parameter is not being used within the method to handle cancellation.

Suggested change

public async Task Handle(Shutdown item, CancellationToken cancellationToken = default)

await Task.Delay(0, cancellationToken);

codecov · 2025-01-07T17:22:23Z

Codecov Report

Attention: Patch coverage is 33.33333% with 36 lines in your changes missing coverage. Please review.

Project coverage is 68.34%. Comparing base (50b1721) to head (11558c2).

Files with missing lines	Patch %	Lines
...togen_ext/runtimes/grpc/protos/agent_worker_pb2.py	4.16%	23 Missing ⚠️
..._ext/runtimes/grpc/protos/agent_worker_pb2_grpc.py	50.00%	12 Missing ⚠️
...ext/runtimes/grpc/_worker_runtime_host_servicer.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4920      +/-   ##
==========================================
- Coverage   68.37%   68.34%   -0.04%     
==========================================
  Files         157      157              
  Lines       10078    10102      +24     
==========================================
+ Hits         6891     6904      +13     
- Misses       3187     3198      +11

Flag	Coverage Δ
unittests	`68.34% <33.33%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jackgerrits · 2025-01-07T20:01:01Z

protos/agent_worker.proto

+    repeated string events = 3;
+    repeated string topics = 4;


Can you explain what these are for?

allows for complete subscription management from registration vs after I think - I've asked Kosta to comment. this was part of his refactor.

Yes, at the point of registration we (at least on the .net side) know all of the events/topics the agent is interested in, no reason to do separate grpc calls to pass that information.

jackgerrits · 2025-01-07T20:01:38Z

protos/agent_worker.proto

        RegisterAgentTypeRequest registerAgentTypeRequest = 4;
        RegisterAgentTypeResponse registerAgentTypeResponse = 5;
-        AddSubscriptionRequest addSubscriptionRequest = 6;
-        AddSubscriptionResponse addSubscriptionResponse = 7;
+        SubscriptionRequest SubscriptionRequest = 6;
+        SubscriptionResponse SubscriptionResponse = 7;


If registration and subscription is to be done with direct RPC then these message can be removed

I assumed we wanted to support both for the time being this late in the game. does that make sense @jackgerrits?

In my refactor, those are removed.

rysweet added 20 commits December 17, 2024 13:49

wip

5f4d1dd

wip working on grpcgateway

56e47cf

worker changes

0ac819c

wip

dfc62d4

proto updates

70d488c

registry grain cleanup

22946a0

core services build

43ed890

Adding AgentTests and host builder cleanup

98e517d

fixing what the tests found

ac7bde8

EventTypes->AgentMetadata

5ee0db8

initialize test

bd66642

more fixes along the way

94ddacb

improve the error for uninitialized agents

e3db41e

move message extensions and update AgentWorker

c7cf200

getting it all to build

e42123c

start mustnt wait

823019d

add unsubscribe

b501d25

adding tests for agentstate

83f5fc0

publish and receive test

acaf3d3

Improving the subscription tests timing

1d6e443

rysweet linked an issue Jan 7, 2025 that may be closed by this pull request

bring over the tests from pr 4405 #4744

Open

rysweet requested a review from Copilot January 7, 2025 17:20

Merge branch 'main' into rysweet-4744-bring-over-the-tests-from-pr-4405

4649d90

Copilot AI reviewed Jan 7, 2025

View reviewed changes

rysweet added 2 commits January 7, 2025 11:29

tests pass now

7d4308f

rename some wierd methods

2e1d24f

jackgerrits reviewed Jan 7, 2025

View reviewed changes

remove unused.

b69bbb5

rysweet added 2 commits January 7, 2025 12:11

adding test project for grpc core

1d58323

update AddGrpcAgentWorker

bf907aa

rysweet marked this pull request as draft January 8, 2025 01:41

rysweet added 6 commits January 7, 2025 17:44

starting to fix the grpcagentworker and gateway

a2e6ca0

format

9fc15f3

format

d69a7da

format

ad94502

Merge branch 'main' into rysweet-4744-bring-over-the-tests-from-pr-4405

54bf9fc

seems like there was a typo

11558c2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rysweet-unsubscribe-and-agent-tests-4744 #4920

rysweet-unsubscribe-and-agent-tests-4744 #4920

rysweet commented Jan 7, 2025

Copilot AI left a comment

Copilot AI Jan 7, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Copilot AI Jan 7, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Copilot AI Jan 7, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

codecov bot commented Jan 7, 2025 •

edited

Loading

jackgerrits Jan 7, 2025

rysweet Jan 8, 2025

kostapetan Jan 8, 2025

jackgerrits Jan 7, 2025

rysweet Jan 8, 2025

kostapetan Jan 8, 2025

	public async Task Handle(NewMessageReceived item, CancellationToken cancellationToken = default)
	var response = await SayHello(item.Message, cancellationToken).ConfigureAwait(false);

	public async Task Handle(ConversationClosed item, CancellationToken cancellationToken = default)
	State = await ReadAsync<AgentState>(this.AgentId, cancellationToken).ConfigureAwait(false);

	public async Task Handle(Shutdown item, CancellationToken cancellationToken = default)
	await Task.Delay(0, cancellationToken);

rysweet-unsubscribe-and-agent-tests-4744 #4920

Are you sure you want to change the base?

rysweet-unsubscribe-and-agent-tests-4744 #4920

Conversation

rysweet commented Jan 7, 2025

Why are these changes needed?

Related issue number

Checks

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot AI Jan 7, 2025

Choose a reason for hiding this comment

Copilot AI Jan 7, 2025

Choose a reason for hiding this comment

Copilot AI Jan 7, 2025

Choose a reason for hiding this comment

codecov bot commented Jan 7, 2025 • edited Loading

Codecov Report

jackgerrits Jan 7, 2025

Choose a reason for hiding this comment

rysweet Jan 8, 2025

Choose a reason for hiding this comment

kostapetan Jan 8, 2025

Choose a reason for hiding this comment

jackgerrits Jan 7, 2025

Choose a reason for hiding this comment

rysweet Jan 8, 2025

Choose a reason for hiding this comment

kostapetan Jan 8, 2025

Choose a reason for hiding this comment

codecov bot commented Jan 7, 2025 •

edited

Loading