feat(autofix): List directory tool operates more like ls #890
Conversation
| repo_client = self.context.get_repo_client(repo_name=repo_name) | ||
|
|
||
| all_paths = repo_client.get_index_file_set(repo_client.get_default_branch_head_sha()) | ||
|
|
||
| paths = [p for p in all_paths if p.startswith(path)] | ||
| # Normalize the path |
There was a problem hiding this comment.
Why do we set the path to the empty string if it doesn't have any trailing/leading slashes?
(sorry looks like I highlighted the snippets incorrectly in my review)
There was a problem hiding this comment.
path.strip("/") should return the path without any trailing/leading slashes, so normalized_path will only be an empty string if path only contains the / character, such as "/"
There was a problem hiding this comment.
I see, I was thinking about cases such as "x/y/z" where there are no leading/trailing slashes, but it's still a valid path. But I guess that case wouldn't occur.
src/seer/automation/autofix/tools.py
Outdated
|
|
||
| return paths_str | ||
| # Remove duplicates and sort | ||
| unique_children = sorted(set(direct_children)) |
There was a problem hiding this comment.
Instead of casting direct_children above to a set, why not just initialize direct_children through set comprehension instead of list comprehension?
| repo_client = self.context.get_repo_client(repo_name=repo_name) | ||
|
|
||
| all_paths = repo_client.get_index_file_set(repo_client.get_default_branch_head_sha()) | ||
|
|
||
| paths = [p for p in all_paths if p.startswith(path)] | ||
| # Normalize the path |
There was a problem hiding this comment.
I see, I was thinking about cases such as "x/y/z" where there are no leading/trailing slashes, but it's still a valid path. But I guess that case wouldn't occur.
Noticed confusion from the LLM agents with the list directory tool. Improved it to list subdirectories and files like how
lsworks as the LLM agents should be much more familiar with the use.This also fixes the bug where an empty string returning the entire git tree.