Skip to content
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

feature(platform) Smart Decision Maker Block #9490

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from

Conversation

Swiftyos
Copy link
Contributor

@Swiftyos Swiftyos commented Feb 17, 2025

Task

The SmartDecisionMakerBlock is a specialized block in a graph-based system that leverages a language model (LLM) to make intelligent decisions about which tools or functions to invoke based on a user-provided prompt. It is designed to process input data, interact with a language model, and dynamically determine the appropriate tools to call from a set of available options, making it a powerful component for AI-driven workflows.

How It Works in Practice

  • Scenario: Imagine a workflow where a user inputs, "Send an email to John about the meeting." The SmartDecisionMakerBlock is connected to tools like send_email, schedule_meeting, and search_contacts.
  • Execution:
    1. The block receives the prompt and system instructions (e.g., "Choose a function to call").
      2.It identifies the available tools from the graph and constructs their signatures (e.g., send_email(recipient, subject, body)).
    2. The LLM analyzes the prompt and decides to call send_email with arguments like recipient: "John", subject: "Meeting", body: "Let’s discuss...".
    3. The block yields these tool-specific outputs, which can be picked up by downstream nodes to execute the email-sending action.

Changes 🏗️

  • Add the Smart Decision Maker (SDM) block.
  • Break circular imports in integration code.

Screenshot 2025-02-21 at 10 23 25

Work in Progress

⚠️ Important note this is a temporary UX for the system - UX will be addressed in a future PR ⚠️

Current Status

I’m currently focused on the smart decision logic. The main additions in the ongoing PR include:

  • Defining function signatures for OpenAI function-calling schemas based on node links and the linked blocks.
  • Adding tests for function signature generation.
  • Force all tool calls to be made via an agent. (Need to uncomment)
  • Restrict each tool call entry to a single node.
  • simplify the output emission process, to emit each parameter one at a time.
  • Change test to use agents and hardcode output how I think it should work to test it does actually work
  • Hook up openai, in a simplified way, to test the function calling (mock for testing)
  • Once all the above is working, use credentials system and build of llm.py

What’s Next

  • Review Process

Reviewers Phase 1

This PR is now ready for review, during the first phase of reviews I'm looking for comments on approach and logic.

Out of scope: code style and organization at this stage

Reviewers Phase 2

Once we are all happy with the approach and logic. We can open the review process to general code quality and nits, to be considered.

Copy link

supabase bot commented Feb 17, 2025

This pull request has been ignored for the connected project bgwpwdsxblryihinutbx because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Copy link
Contributor

This PR targets the master branch but does not come from dev or a hotfix/* branch.

Automatically setting the base branch to dev.

@github-actions github-actions bot changed the base branch from master to dev February 17, 2025 10:29
@Swiftyos Swiftyos changed the title Swiftyos/open 2373 tool discovery input mapping for smart decision maker block feature(platform) Smart Decision Maker Block Feb 17, 2025
Copy link

netlify bot commented Feb 17, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 267b96d
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67b8a1a43c576d000875bd31

Copy link

deepsource-io bot commented Feb 17, 2025

Here's the code health analysis summary for commits a692eed..267b96d. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ Success
❗ 2 occurences introduced
🎯 2 occurences resolved
View Check ↗
DeepSource Python LogoPython✅ Success
❗ 12 occurences introduced
🎯 2 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

netlify bot commented Feb 17, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 267b96d
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67b8a1a45bd5df000883b0f9

if not graph:
raise ValueError(f"Graph not found {graph_id}")

tool_functions = self._create_function_signature(node_id, graph)
Copy link
Member

Choose a reason for hiding this comment

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

Test this works with mem0

since it uses the kwargs based on its place in the graph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ntindle It seems that high level decision is that tools must be an AgentExecutionBlock.

If you feel strongly that this is a bad idea, let me know and we can discuss it with Toran

Copy link
Member

Choose a reason for hiding this comment

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

Why should it require that? I can see plenty of use for this not needing a whole agent and the overhead that would come with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main reason why is to make a smart decision, a certain amount of information is required about a tool. We can backprogate this information from an agent - name, description, required params, etc.

But with a normal block, we can't really do this unless we assume that block can't be connected to another block...

There are a fair bit of subtleties with this system

@github-actions github-actions bot added size/xl and removed size/l labels Feb 19, 2025
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Feb 20, 2025
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

…l-discovery-input-mapping-for-smart-decision-maker-block
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Feb 20, 2025
@github-actions github-actions bot added the platform/frontend AutoGPT Platform - Front end label Feb 21, 2025
@Swiftyos
Copy link
Contributor Author

@Pwuts tests are failing due to an issue with the credentials system that does not occur locally. Can you advise on how to fix this please

@Swiftyos Swiftyos marked this pull request as ready for review February 21, 2025 09:33
@Swiftyos Swiftyos requested a review from a team as a code owner February 21, 2025 09:33
@Swiftyos Swiftyos requested review from Pwuts and majdyz and removed request for a team February 21, 2025 09:33
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns

API Key Exposure:
The error handling in llm_call() function includes API keys in error messages (e.g., "Anthropic API error: {str(e)}"). This could lead to sensitive API keys being exposed in logs or error responses. The error messages should be sanitized to remove any sensitive information before logging or returning to users.

⚡ Recommended focus areas for review

Error Handling

The LLM response handling in the SmartDecisionMakerBlock needs validation to ensure proper error handling when the LLM returns invalid or unexpected responses.

    credentials=credentials,
    llm_model=input_data.model,
    prompt=prompt,
    json_format=False,
    max_tokens=input_data.max_tokens,
    tools=tool_functions,
    ollama_host=input_data.ollama_host,
)

if not response.tool_calls:

    yield "finished", f"No Decision Made finishing task: {response.response}"
Security Risk

The llm_call function exposes API keys in error messages which could lead to sensitive information disclosure in logs.

except anthropic.APIError as e:
    error_message = f"Anthropic API error: {str(e)}"
    logger.error(error_message)
    raise ValueError(error_message)
Input Validation

The validate_graph method needs to verify that tool names are unique across the entire graph to prevent naming conflicts.

tool_name = link.source_name.split("_#_")[0]
if tool_name in tool_name_to_node:
    if tool_name_to_node[tool_name] != link.sink_id:
        raise ValueError(
            f"Tool name {tool_name} links to multiple nodes: {tool_name_to_node[tool_name]} and {link.sink_id}"
        )
else:
    tool_name_to_node[tool_name] = link.sink_id

@Swiftyos Swiftyos requested a review from ntindle February 21, 2025 09:35
Copy link
Contributor

@majdyz majdyz left a comment

Choose a reason for hiding this comment

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

Are these true:

  • SMD can't be connected to non-AgentExecutorBlock, if so, why not? Would it calling non-agent block would be simpler, e.g: We have a block definition, type, description, etc, plus it's a more common pattern.

  • the tools output pin will connected to N other input pins, and then on each yield, N input will be produced, but N-1 of it will raise an exception due to the validation failure since it doesn't match the tool that the SMD is using. Is this the flow we are expecting ?

prompt: list[dict],
json_format: bool,
max_tokens: int | None,
tools: list[dict] | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is tools here how do I use it? can we add it to the docstring too?

completion_tokens: int


def llm_call(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is copied function, can we remove the old one and make the function caller use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really want too, but I know how much people hate me making bigger PRs. So I was planning on refactoring llm.py in a separate PR if thats okay.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay sounds good, though if the change is just removing self. in self.llm_call and removing the llm_call function I definitely don't mind doing it in a single PR. Both are fine for me 👍

tool_links = {
link.sink_id
for link in graph.links
if link.source_name.startswith("tools_") and link.source_id == node_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make tools_ constant, and this is so prone to conflict, can we use a special character?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure good idea

for link in self.links:
input_links[link.sink_id].append(link)

if (
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comments here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants