Skip to content

feat: Standardize AgentREPL handling of output; provide default prompt#458

Merged
eob merged 14 commits into
mainfrom
repl-hook
Jul 11, 2023
Merged

feat: Standardize AgentREPL handling of output; provide default prompt#458
eob merged 14 commits into
mainfrom
repl-hook

Conversation

@eob
Copy link
Copy Markdown
Contributor

@eob eob commented Jul 6, 2023

This PR attempts to simplify the complexity of developing & creating an Agent in two ways:

  1. It standardizes the handling of Tool/Agent output within the Repl (previously Tool output was handled by the REPL, but Agent output was handled by the Agent).

  2. It provides a default prompt method if none is provided during REPL instantiation. This permits the 90% case of agent development to be nothing more than the agent definition itself, without the prompt method which sets up the debugging harness.

@eob eob requested review from dkolas and douglas-reid July 6, 2023 04:23
dkolas
dkolas previously approved these changes Jul 6, 2023
Copy link
Copy Markdown
Contributor

@dkolas dkolas left a comment

Choose a reason for hiding this comment

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

Looks good, though I wonder about the repl model thing below.

Comment thread src/steamship/utils/repl.py Outdated

# Use either "gpt-3.5-turbo-0613" or "gpt-4-0613" here.
# Other versions of GPT tend not to work well with the ReAct prompt.
MODEL_NAME = "gpt-4-0613"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this have to be defined in the REPL? Is there a way to refactor around this? It feels really strange that the REPL would be selecting a default model.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is only there because:

  • I've moved the prompt method into the REPL so that every single AgentService doesn't have to define it's own testing harness, and
  • it makes which model we're using explicit

Should we just do it as an inline string down where the LLM is created? That would accomplish the same thing, but without calling it out with a constant like that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the motivation for removing this one? did it go somewhere else? This seems like a very relevant example;
I've pointed people at it a couple times in the last few days

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I agree -- I added it back just now

Copy link
Copy Markdown
Contributor

@douglas-reid douglas-reid left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me. The only thing I think we need to do is to add some discussion of AgentContext and how to create one, etc., somewhere. The prompt() testing bits covered that (and also introduced a bunch of confusion), but it would be great if we had a way to provide the same documentation somewhere convenient.

@eob
Copy link
Copy Markdown
Contributor Author

eob commented Jul 7, 2023

@douglas-reid yeah, I agree that once of the side benefits to having it in the agents was that it acted as a kind of documentation.

I'll do another pass to see if I can find a way to work that documentation in there -- or at least clear links to it -- before going for a merge.

The REPL is also printing Block(UUID) instead of the actual URL, when I generate images from agents, which makes me think I've missed something I'd intended in the output refactoring too.

Comment thread src/steamship/agents/service/agent_service.py
Comment thread src/steamship/agents/service/agent_service.py
@eob
Copy link
Copy Markdown
Contributor Author

eob commented Jul 11, 2023

All the tests passed; the one that failed was a ghost in the system of an unrelated test, so going to merge.

@eob eob merged commit 3c572d6 into main Jul 11, 2023
@eob eob deleted the repl-hook branch July 11, 2023 19:12
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.

3 participants