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

Usability improvements #1191

Merged
merged 9 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions gpt_engineer/applications/cli/file_selector.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great idea! We're implementing this to save user time by skipping file selection. This will be included in a uniform config file where users can set all configurations, including the model. Sorting files is a good suggestion. Please add tests for these features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests:

  • tests/core/test_file_selector_enhancements.py: test_file_selector_enhancement_skip_file_selector
  • tests/core/test_file_selector_enhancements.py: test_file_selector_enhancement_sort

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding the tests for the file selector enhancements. I appreciate your thoroughness in testing both skipping and sorting.

Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def __init__(self, project_path: Union[str, Path]):
self.metadata_db = DiskMemory(metadata_path(self.project_path))
self.toml_path = self.metadata_db.path / self.FILE_LIST_NAME

def ask_for_files(self) -> tuple[FilesDict, bool]:
def ask_for_files(self, skip_file_selection=False) -> tuple[FilesDict, bool]:
"""
Prompts the user to select files for context improvement.

Expand All @@ -89,8 +89,9 @@ def ask_for_files(self) -> tuple[FilesDict, bool]:
A dictionary with file paths as keys and file contents as values.
"""

if os.getenv("GPTE_TEST_MODE"):
if os.getenv("GPTE_TEST_MODE") or skip_file_selection:
# In test mode, retrieve files from a predefined TOML configuration
# also get from toml if skip_file_selector is active
assert self.FILE_LIST_NAME in self.metadata_db
selected_files = self.get_files_from_toml(self.project_path, self.toml_path)
else:
Expand Down Expand Up @@ -412,7 +413,7 @@ def get_current_files(self, project_path: Union[str, Path]) -> List[str]:
if is_git_repo(project_path) and "projects" not in project_path.parts:
all_files = filter_by_gitignore(project_path, all_files)

return all_files
return sorted(all_files, key=lambda x: Path(x).as_posix())


class DisplayablePath(object):
Expand Down
12 changes: 11 additions & 1 deletion gpt_engineer/applications/cli/main.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good location for user interface functions. We plan to move most functions into the config file. Please add tests for these features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test:

  • tests/application/cli/test_main.py:test_improve_existing_project_skip_file_selection

Copy link
Collaborator

Choose a reason for hiding this comment

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

Valuable enhancement. Thanks for covering this aspect.

Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ def main(
"--use_cache",
help="Speeds up computations and saves tokens when running the same prompt multiple times by caching the LLM response.",
),
skip_file_selection: bool = typer.Option(
False,
"--skip-file-selection",
"-s",
help="Skip interactive file selection in improve mode and use the generated TOML file directly.",
),
no_execution: bool = typer.Option(
False,
"--no_execution",
Expand Down Expand Up @@ -405,6 +411,8 @@ def main(
Speeds up computations and saves tokens when running the same prompt multiple times by caching the LLM response.
verbose : bool
Flag indicating whether to enable verbose logging.
skip_file_selection: bool
Skip interactive file selection in improve mode and use the generated TOML file directly
no_execution: bool
Run setup but to not call LLM or write any code. For testing purposes.
sysinfo: bool
Expand Down Expand Up @@ -501,7 +509,9 @@ def main(
files = FileStore(project_path)
if not no_execution:
if improve_mode:
files_dict_before, is_linting = FileSelector(project_path).ask_for_files()
files_dict_before, is_linting = FileSelector(project_path).ask_for_files(
skip_file_selection=skip_file_selection
)

# lint the code
if is_linting:
Expand Down
2 changes: 1 addition & 1 deletion gpt_engineer/core/chat_to_files.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

The increased timeout from 1 to 10 seconds improves robustness but may excessively prolong execution. Consider a shorter timeout (e.g., 3-5 seconds) to balance reliability and performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defaulted to 3 secs.
Added a new command line parameter "--diff_timeout INTEGER" to override.
Also created some test in tests/application/cli/test_main.py:test_improve_existing_project_diff_timeout for the new command line option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great work on defaulting time and adding the new command line parameter for overriding. The test you created for this is a nice addition.

Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def parse_diffs(diff_string: str) -> dict:

diffs = {}
try:
for block in diff_block_pattern.finditer(diff_string, timeout=1):
for block in diff_block_pattern.finditer(diff_string, timeout=10):
diff_block = block.group()

# Parse individual diff blocks and update the diffs dictionary
Expand Down
3 changes: 2 additions & 1 deletion gpt_engineer/tools/supported_languages.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good suggestion to support multiple languages. We will keep this feature as suggested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also added Arduino *.ino files. They are basically C++ with some extra functions like "setup()" and "loop()" - works well with ChatGPT & DeepSeek.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Including the Arduino *.ino files was a smart move. It's good to know they integrate well with ChatGPT & DeepSeek.

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"extensions": [".cpp", ".cc", ".cxx", ".h", ".hpp", ".hxx"],
"tree_sitter_name": "cpp",
},
{"name": "C", "extensions": [".c", ".h"], "tree_sitter_name": "c"}
{"name": "C", "extensions": [".c", ".h"], "tree_sitter_name": "c"},
{"name": "Markdown", "extensions": [".md"], "tree_sitter_name": "md"}
# ---- the following are not supported by the current code chunker implementation ----
# {
# "name": "Swift",
Expand Down
Loading