refactor: cleanup agent class objects#384
Conversation
eob
left a comment
eob
left a comment
There was a problem hiding this comment.
Looks really solid. A few minor comments within
|
|
||
| self.tags.append(tag) | ||
|
|
||
| def as_llm_input(self) -> str: |
| _LLM_KEY = "llm" | ||
|
|
||
|
|
||
| def with_llm(llm: LLM, context: Optional[AgentContext] = None) -> AgentContext: |
There was a problem hiding this comment.
From the PR message it sounds like this felt mostly like an odd shim to write, but I actually think this style may prove super useful as a way to think....
If things like "Which LLM am I using" become dynamic settings of an instance-in-progress (e.g. 'let's try Cohere out'), then this kind of chaining could let us pile on user settings atop reasonable defaults in a pretty elegant way
| while not isinstance(action, FinishAction): | ||
| action = self.take_action(context) | ||
|
|
||
| # TODO: logging? |
There was a problem hiding this comment.
Check out my weird, but I think workable (?), logging PR -- it proposes using structured logging to let us lift different views about agent/tool activity out of the standard logs.
| pass | ||
|
|
||
|
|
||
| class Tool(BaseModel): |
There was a problem hiding this comment.
Does this need to include ABC since below has a @AbstractMethod?
| """Any direct output produced by the Tool.""" | ||
|
|
||
|
|
||
| class AgentTool(Tool): |
There was a problem hiding this comment.
nit/readability - I was confused by AgentTool as a name.
I think the idea is that we want Tool to be an ABC, but we also want something that suffices as a short-cut for NoOp?
Maybe we just call it NoOpTool (if that's what it is)?
| from steamship.agents.schema.tool import AgentContext, Tool | ||
|
|
||
|
|
||
| class Action(BaseModel): |
There was a problem hiding this comment.
Not important now, but thought pin to wonder if eventually input and output will benefit from being a proxy object which represents: "The ability to load blocks -- now or in the future". Such that both bindings can be a persistence-friendly future on blocks.
dkolas
left a comment
dkolas
left a comment
There was a problem hiding this comment.
You are the hero we need, not the hero we deserve
| """Any direct output produced by the Tool.""" | ||
|
|
||
|
|
||
| class AgentTool(Tool): |
3 participants
This PR represents an attempt to cleanup, document, and rationalize the existing set of classes in
agenthierarchy. I also wanted to remove as much vestigial and unused surface (primarily related to async execution) in an attempt to simplify and reduce surface area for implementors.As part of this PR, I updated the example assistant to show how synchronous agent responses could be done using
emitFns. This then led to an update of the REPL to allow flexible identification ofAgentServicemethods. I think we need more thought there.Because we've introduced an
LLMclass, I took theget_llm()method that created ad-hoc ones from context off of the context and added helpers to create contexts with LLM instances. This was an attempt to rationalize usage within Tools, etc. I'm open to other approaches there.