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

Remove config from command decorator #4736

Conversation

waynehamadi
Copy link
Contributor

@waynehamadi waynehamadi commented Jun 19, 2023

Background

This piece of code is making a call to the config singleton and we want to get rid of this singleton.
This code is only useful to prevent use of a command if the AI decides to use it. But it's very unlikely it decides to use it because it's not even in the prompt, since this piece of code makes sure of it:
autogpt/prompts/generator.py

if item_type == "command":
            command_strings = []
            if self.command_registry:
                command_strings += [
                    str(item)
                    for item in self.command_registry.commands.values()
                    if item.enabled => HERE
                ]
            # terminate command is added manually
            command_strings += [self._generate_command_string(item) for item in items]
            return "\n".join(f"{i+1}. {item}" for i, item in enumerate(command_strings))

Changes

  • Remove the call to config() in command decorator.

Documentation

Test Plan

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.
  • I have run the following commands against my code to ensure it passes our linters:
    black .
    isort .
    mypy
    autoflake --remove-all-unused-imports --recursive --ignore-init-module-imports --ignore-pass-after-docstring autogpt tests --in-place

@netlify
Copy link

netlify bot commented Jun 19, 2023

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 6679109
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/648fb9f71a11320008de6edd

@github-actions
Copy link
Contributor

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

@waynehamadi waynehamadi force-pushed the remove-config-from-command branch from 6a3532a to 0c26d73 Compare June 19, 2023 01:15
@github-actions
Copy link
Contributor

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

erik-megarad
erik-megarad previously approved these changes Jun 19, 2023
@waynehamadi waynehamadi dismissed erik-megarad’s stale review June 19, 2023 02:07

The merge-base changed after approval.

@waynehamadi waynehamadi force-pushed the remove-config-from-command branch from 0c26d73 to f46b209 Compare June 19, 2023 02:07
@github-actions github-actions bot added size/m and removed size/xl labels Jun 19, 2023
@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 Jun 19, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.14 ⚠️

Comparison is base (a7f8056) 71.03% compared to head (6679109) 70.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4736      +/-   ##
==========================================
- Coverage   71.03%   70.90%   -0.14%     
==========================================
  Files          68       68              
  Lines        3235     3227       -8     
  Branches      516      512       -4     
==========================================
- Hits         2298     2288      -10     
+ Misses        774      773       -1     
- Partials      163      166       +3     
Impacted Files Coverage Δ
autogpt/command_decorator.py 100.00% <ø> (+4.34%) ⬆️

... and 5 files with indirect coverage changes

☔ 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.

@ntindle
Copy link
Member

ntindle commented Jun 19, 2023

Check to make sure disabled commands aren't being registered in the prompt with this removed. This stayed for a reason when I refactored

@erik-megarad erik-megarad merged commit 0abfa3a into Significant-Gravitas:master Jun 19, 2023
@waynehamadi
Copy link
Contributor Author

it's not there I checked.

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

Successfully merging this pull request may close these issues.

4 participants