Skip to content

SK using autogen core process runtime #2

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

moonbox3
Copy link
Owner

Motivation and Context

WIP. Sample Only.

Description

Contribution Checklist

super().__init__(description, kernel, process, parent_process_id=parent_process_id)
self.kernel = kernel
self.process = process
self.ag_runtime = runtime
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW - Isn't runtime available from BaseAgent? Might not need duplicate ag_runtime.

if not process.state.name:
raise ValueError("Process state name must not be empty")

self.runtime = SingleThreadedAgentRuntime()
Copy link

@crickman crickman Feb 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this is POC. In a more full design, the runtime would be provided as a parameter here (or somehow pluggable).

description: str,
kernel: Kernel,
process: KernelProcess,
runtime: SingleThreadedAgentRuntime,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typing this as a SingleThreadedAgentRuntime is a side-effect of the short-cut taken with AGKernelProcessContext.

"""Starts the process with an initial event."""
await self.ensure_initialized()
self.process_task = asyncio.create_task(self.internal_execute(keep_alive=keep_alive))
self.ag_runtime.start()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting the runtime here is a only coincidentally possible because you've captured ag_runtime as SingleThreadedAgentRuntime. In a more fully considered design, runtime initialize would likely occur outside of the process. (There might likely be special considerations for starting different types of runtimes).

return self.step_info.state.name

@property
def type(self) -> str:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting that type is being overloaded to mean id (AgentId defined as a tuple of type and id, but since type === id there is only going to be a single agent of any "type". I'm pretty sure this is coloring outside of the lines w.r.t. the intended usage pattern. In AG .NET "type" has wiring that ties it to System.Type; although, under the scenes its still a string.

Its clever how you've hacked the runtime here...I'm not against coloring outside of the lines, but I wonder if there's an insight to be learned here as we generalize the runtime. I'd be curious to get Jack's read on this innovative approach.

parent_process_id=self.id.key if isinstance(self.id, AgentId) else None,
)

agent_id = AgentId(f"{step_info.state.id}", step_info.state.id)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Another instance of overloading "type" with the identifier.

logger.info(f"[AGProcessAgent] Registering process agent: {self.proc_agent_id}")

await self.runtime.register_factory(
type=self.proc_agent_id.type,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another instance of the "type" being overloaded with the "id"

self.process = process

proc_id = process.state.id or uuid.uuid4().hex
self.proc_agent_id = AgentId(f"{proc_id}", proc_id)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another instance of the "type" being overloaded with the "id"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, feels like this logic (defining type/id and registering process) might be reconciled with the logic inside of the process-agent. (Not sure it needs to be handled within the context as a special case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants