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

Add navigation commands into the editor #5

Closed
wants to merge 43 commits into from
Closed

Conversation

ryanhoangt
Copy link
Collaborator

@ryanhoangt ryanhoangt commented Nov 5, 2024

Description

This PR is to add navigation commands into the editor:

  • jump_to_definition
  • find_references

The 2 commands will be enabled when detecting a git repo in cwd. Otherwise, it will print:

No git repository found. Navigation commands are disabled. Please use bash commands instead.

Also, if the navigator commands are enabled, when the agent views a file, the content is also appended with to encourage exploration. If no git repo found the TIP will not be appended.

<TIP>Use the `jump_to_definition` and `find_references` commands to understand more about a particular class, function/method and how it is used in the codebase.</TIP>

How Has This Been Tested?

symbol-navigator.1.mov

@ryanhoangt ryanhoangt marked this pull request as ready for review November 23, 2024 04:40
@ryanhoangt ryanhoangt requested a review from xingyaoww November 23, 2024 04:48
@ryanhoangt
Copy link
Collaborator Author

Hey @xingyaoww, I think this PR is mostly ready. Some remaining issues I'm not sure how to handle properly yet:

  • The tree-sitter parser uses a .oh_aci.cache.tags directory for caching purpose, which means it'll create this directory in the workspace. This might be a bit annoying, and I'm thinking about removing this cache dir, as the command itself is not used very frequently by the agent. But with repomap we should find some ways to deal with this, as the repomap is constructed for every step of execution.
  • In the GitRepoUtils class, I'm doing some traversal to detect the git repo in the workspace. But if under workspace there're multiple git repos, the current implementation may pick up the wrong one.
    Do you have some suggestions on these?

@xingyaoww
Copy link
Collaborator

The tree-sitter parser uses a .oh_aci.cache.tags directory for caching purpose, which means it'll create this directory in the workspace. This might be a bit annoying, and I'm thinking about removing this cache dir, as the command itself is not used very frequently by the agent. But with repomap we should find some ways to deal with this, as the repomap is constructed for every step of execution.

Is there a way for us to create this directory elsewhere? Say something under /tmp?

In the GitRepoUtils class, I'm doing some traversal to detect the git repo in the workspace. But if under workspace there're multiple git repos, the current implementation may pick up the wrong one. Do you have some suggestions on these?

Hmm interesting, can you point me to the exact code for this? I'm thinking we could potentialy track back from an model given path?
Say the model want to view /workspace/a/b/c.py - we first check if b is a git repo, if not, we go to a?

@ryanhoangt
Copy link
Collaborator Author

Is there a way for us to create this directory elsewhere? Say something under /tmp?

Yeah thanks for the idea, I think we can do this.

Hmm interesting, can you point me to the exact code for this? I'm thinking we could potentialy track back from an model given path?
Say the model want to view /workspace/a/b/c.py - we first check if b is a git repo, if not, we go to a?

Below is the code, I looked into command output in the trajectory and seems like when the file_editor script is run, the cwd (./ -- default value passed to GitRepoUtils) resolves to /workspace, which is often not the actual workspace location that the user sets later. I'm not sure if there's a way to communicate the actual workspace location set by user to the file_editor plugin? If yes we won't need to do the guessing here.

def __init__(self, abs_repo_path: str) -> None:
from git import Repo
if not Path(abs_repo_path).is_absolute():
raise ValueError('The path must be absolute')
self.repo_path = Path(abs_repo_path)
try:
self.repo = Repo(self.repo_path)
except Exception:
logger.warning(
f'Could not find git repository at {abs_repo_path}, trying to detect at child directories'
)
for child in self.repo_path.iterdir():
if child.is_dir():
try:
self.repo = Repo(child)
self.repo_path = child
logger.info(f'Found git repository at {child}')
break
except Exception:
logger.warning(
f'Could not find git repository at child {child}'
)
if not self.repo:
raise Exception(
'Could not find any git repository in the root directory or its children'
)

@ryanhoangt
Copy link
Collaborator Author

@xingyaoww I figured out a way to handle the workspace thing, thanks to the pwd already set to bash pwd for ipython. This should be finally ready. Please take another look, then I'll cut a release and finish up the PR in OH repo.

openhands_aci/editor/navigator.py Outdated Show resolved Hide resolved
openhands_aci/editor/navigator.py Outdated Show resolved Hide resolved
openhands_aci/editor/navigator.py Outdated Show resolved Hide resolved
openhands_aci/editor/navigator.py Show resolved Hide resolved
query = ts_language.query(tags_query)
captures = query.captures(parsed_tree.root_node)

parsed_tags = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you call it tags? Seems a bit confusing? Is it just node?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a term in tree-sitter when using query files, the output can be called tag, which is just a named entity, or syntax node: https://tree-sitter.github.io/tree-sitter/code-navigation-systems

@xingyaoww
Copy link
Collaborator

lmk when this is ready for another look!

@ryanhoangt
Copy link
Collaborator Author

Close this in favor of #35.

@ryanhoangt ryanhoangt closed this Dec 26, 2024
@ryanhoangt ryanhoangt deleted the ht/jump-commands branch January 3, 2025 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants