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

Improve command system; add aliases for commands #2635

Merged

Conversation

lengweiping1983
Copy link
Contributor

@lengweiping1983 lengweiping1983 commented Apr 20, 2023

Note This PR has been "hijacked" and expanded by a maintainer to further increase its value and accelerate its integration.

Background

The AI will regularly hallucinate command names. By providing probable synonyms (aliases) for existing commands, we can prevent failure in such cases.

Before:

@command(
    "write_to_file",
    "Write to file",
    '"filename": "<filename>", "text": "<text>"',
)

Now:

@command(
    "write_to_file",
    "Write to file",
    '"filename": "<filename>", "text": "<text>"',
    name_alias=["write_file", "create_file"],
)

write_to_file, write_file, and create_file will all take effect.

Changes

  • CommandRegistry
    • Add __contains__() method to support checking command_name in registry
    • Use commands_aliases to map aliases to commands in get_command(), call() and __contains__()
    • unregister() now takes a Command as its argument instead of str
    • get_command() now returns None if a command does not exist, instead of a KeyError
  • PromptGenerator
    • Fix import error for CommandRegistry
    • Add type hints to attributes
    • args -> params
  • app.py::execute_command()
    • Use CommandRegistry.get_command() instead of CommandRegistry.commands.get()
    • Improve comments to clarify code flow and purpose
  • Rename app.py::get_command() to extract_command to clarify its function
  • test_prompt_generator.py
    • Update tests in accordance with changes to PromptGenerator
    • Delete exact duplicate test test_generate_prompt_string
  • test_commands.py
    • Convert to pytest-style
      • Add fixtures to reduce duplicate code
    • Reduce overlap between some tests
    • Use command_name in registry instead of command_name in registry.commands
    • Add test_command_in_registry to test in operator

Documentation

x

Test Plan

CI; see changes to tests

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

@lengweiping1983
Copy link
Contributor Author

@nponeccop @BillSchumacher

this PR Extended Command Features.
Command Multiple names are supported, in the form of aliases.

please review. #2635

@github-actions
Copy link

Coverage report

The coverage rate went from 33.74% to 34.1% ⬆️
The branch rate is 21%.

78.26% of new lines are covered.

Diff Coverage details (click to unfold)

autogpt/commands/command.py

80.95% of new lines are covered (79.09% of the complete file).
Missing lines: 71, 79, 87, 88

autogpt/commands/google_search.py

0% of new lines are covered (0% of the complete file).
Missing lines: 14

autogpt/commands/file_operations.py

100% of new lines are covered (62.84% of the complete file).

@lengweiping1983
Copy link
Contributor Author

@Pwuts @nponeccop Today I found I used the company email to submit the PR, now I changed to my personal email and resubmitted.

@lengweiping1983
Copy link
Contributor Author

@BillSchumacher please review this PR, and #2311

@Pwuts Pwuts changed the title command module keep parameter names consistent Add aliases for commands Apr 27, 2023
@Pwuts Pwuts requested a review from Torantulino April 27, 2023 18:45
@p-i-
Copy link
Contributor

p-i- commented May 5, 2023

This is a mass message from the AutoGPT core team.
Our apologies for the ongoing delay in processing PRs.
This is because we are re-architecting the AutoGPT core!

For more details (and for infor on joining our Discord), please refer to:
https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting

@Boostrix Boostrix mentioned this pull request May 5, 2023
5 tasks
@Boostrix
Copy link
Contributor

Boostrix commented May 8, 2023

indeed, the idea looks much better than hard-coding aliases into app.py (see map_command_synonyms there)

@Boostrix Boostrix added this to the v0.4.0 Release milestone May 16, 2023
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label May 17, 2023
@github-actions
Copy link

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

@vercel
Copy link

vercel bot commented May 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 19, 2023 9:59am

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label May 19, 2023
@github-actions
Copy link

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

@vercel vercel bot temporarily deployed to Preview May 19, 2023 09:59 Inactive
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label May 26, 2023
@github-actions
Copy link

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

Copy link
Contributor

@lc0rp lc0rp left a comment

Choose a reason for hiding this comment

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

Interesting feature. Thanks for submitting.

It would be good to have benchmarks in place to prevent any confusion with the LLM and ensure that it doesn't consume an excessive amount of the context window. Additionally, it would be good to consider the synonyms feature. Lastly, for future ref: there were some variable renames that were unnecessary and not relevant to this particular feature.

Please resolve conflicts and thanks again for submitting!

autogpt/commands/command.py Outdated Show resolved Hide resolved
autogpt/commands/command.py Outdated Show resolved Hide resolved
autogpt/commands/command.py Outdated Show resolved Hide resolved
@netlify
Copy link

netlify bot commented Jul 7, 2023

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit d15fd3e
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/64a973791d79180008c1d784

@github-actions github-actions bot added size/xl and removed size/m labels Jul 8, 2023
@github-actions
Copy link

github-actions bot commented Jul 8, 2023

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@github-actions
Copy link

github-actions bot commented Jul 8, 2023

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@Pwuts Pwuts changed the title Add aliases for commands Improve command system; add aliases for commands Jul 8, 2023
@Pwuts Pwuts self-assigned this Jul 8, 2023
@Pwuts Pwuts added this to the v0.4.5 Release milestone Jul 8, 2023
@github-actions
Copy link

github-actions bot commented Jul 8, 2023

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@github-actions
Copy link

github-actions bot commented Jul 8, 2023

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@Auto-GPT-Bot
Copy link
Contributor

You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged.

@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Patch coverage: 88.67% and project coverage change: +0.22 🎉

Comparison is base (8bce027) 50.29% compared to head (d15fd3e) 50.52%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2635      +/-   ##
==========================================
+ Coverage   50.29%   50.52%   +0.22%     
==========================================
  Files         116      116              
  Lines        4843     4857      +14     
  Branches      653      655       +2     
==========================================
+ Hits         2436     2454      +18     
+ Misses       2221     2219       -2     
+ Partials      186      184       -2     
Impacted Files Coverage Δ
autogpt/command_decorator.py 100.00% <ø> (ø)
autogpt/commands/file_operations.py 80.80% <ø> (ø)
autogpt/commands/web_search.py 97.77% <ø> (ø)
autogpt/main.py 23.68% <0.00%> (ø)
autogpt/app.py 41.86% <75.00%> (-5.20%) ⬇️
autogpt/models/command_registry.py 73.68% <82.60%> (+4.93%) ⬆️
autogpt/agent/agent.py 59.18% <100.00%> (ø)
autogpt/models/command.py 73.91% <100.00%> (+1.18%) ⬆️
autogpt/prompts/generator.py 92.98% <100.00%> (+6.31%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Auto-GPT-Bot
Copy link
Contributor

You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged.

@Pwuts Pwuts merged commit 8b8b3a2 into Significant-Gravitas:master Jul 8, 2023
16 checks passed
@Pwuts Pwuts added the code quality ⬆️ PRs that improve code quality label Jul 8, 2023
dayofthedave pushed a commit to dayofthedave/Auto-GPT that referenced this pull request Jul 17, 2023
…s#2635)

* Command name supports multiple names

* Separate CommandRegistry.commands and .command_aliases

* Update test_commands.py

* Add __contains__ operator to CommandRegistry

* Update error message for unknown commands

---------

Co-authored-by: Reinier van der Leer <github@pwuts.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI efficacy behaviour change code quality ⬆️ PRs that improve code quality Needs Benchmark This change is hard to test and requires a benchmark size/xl
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants