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 auto completions #79

Merged
merged 16 commits into from
Sep 5, 2023
Merged

Conversation

waydegg
Copy link
Contributor

@waydegg waydegg commented Aug 26, 2023

Summary

This PR adds completions for syntax (classes, variables, functions, etc.) and file names for the prompt session.

Completions kept in sync as files are added/updated/deleted (they are "refreshed" every 5 seconds atm). Whenever a user starts a prompt with "/" we return command completions only.

Since we need to keep track of what files are in the context for keeping completions up-to-date, I've added a CodeFileIndex class that currently just holds a set of file names added into the context. CodeFileIndex is shared between CodeFileManager and UserInputManager. This is done so that when CodeFileIndex modifies the filesystem, `UserInputManager can see these modifications and make updates to completions.

To-Do

  • Update test cases for CodeFileManager
  • Add test cases for MentatCompleter

@PCSwingle
Copy link
Member

Just fyi, in my commands PR #59 I change how we use prompt toolkit a lot; you might want to work off of that branch until it gets merged.

@waydegg
Copy link
Contributor Author

waydegg commented Aug 29, 2023

@biobootloader would love your thoughts are on CodeFileIndex and if you can think of a more elegant way to share file paths w/ UserInputManager (i.e. less changes required for CodeFileManager)

# 4. Decide on what words to auto complete (right now it's everything)
# a. Use ctags to add types (e.g. function, class) to completions
# 5. Use ctags if available, fallback to pygments lexer if not
# 6. Consider fuzzy matching completions
Copy link
Member

Choose a reason for hiding this comment

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

I love fuzzy matching in general. I guess the downside could be too many options popping up for everything you type?

Rift has an @ syntax for referencing files, maybe something like that could be good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can limit the number of completions shown. Prompt Toolkit has a built-in fuzzy matcher that's pretty fast too.

Yea I like the @ directive for Rift and Cursor. Happy to make that character show file completions specifically.

@biobootloader
Copy link
Member

@biobootloader would love your thoughts are on CodeFileIndex and if you can think of a more elegant way to share file paths w/ UserInputManager (i.e. less changes required for CodeFileManager)

I think it is going to require changes to CodeFileManager. I guess the question is how do we divide responsibilities best between:

  • CodeFileManager
  • CodeMap
  • CodeFileIndex
  • MentatCompleter

Both CodeFileManager and MentatCompleter need to know which files / partial files are in the context, and this will grow more complicated as we add features to Mentat that automatically add partial files to context as the conversation continues. So your approach is to make CodeFileIndex to do that shared work.

Maybe a better name for CodeFileIndex could something like ContextManager? Maybe it should also own get_code_message() (currently in CodeFileManager), and CodeFileManager should only be responsible for reading and writing to files?

Another question is what we should auto-complete - just stuff in context or also stuff that's not in context, like from the CodeMap? I think including stuff that's outside context will be important because we'll want user to be able add files/classes/functions to context easily, within a conversation. But maybe that'd be too many completions being suggested for big codebases? The completion engine should probably know which completions are coming from within or outside context, so it could prioritize things in context.

I'm not really sure about any of this, but those are some initial thoughts!

@biobootloader
Copy link
Member

I don't think this branch has the partial files changes from #69, sorry that that complicates things!

@waydegg
Copy link
Contributor Author

waydegg commented Aug 30, 2023

Maybe a better name for CodeFileIndex could something like ContextManager? Maybe it should also own get_code_message() (currently in CodeFileManager), and CodeFileManager should only be responsible for reading and writing to files?

Yup that sounds good. Make CodeFileManager just handle CRUD actions for files, and ContextManager is where references to files and partial files are stored.

@waydegg
Copy link
Contributor Author

waydegg commented Aug 30, 2023

Another question is what we should auto-complete - just stuff in context or also stuff that's not in context, like from the CodeMap?

Right now it's just stuff inside the context, however I don't think it would hurt to add completions for files/syntax inside git repos? I think we could enable/disable this depending on if the code-map is enabled/disabled.

@waydegg
Copy link
Contributor Author

waydegg commented Aug 30, 2023

I don't think this branch has the partial files changes from #69, sorry that that complicates things!

Good callout! Will fetch from main and update this

@@ -4,3 +4,4 @@ known_first_party = "mentat"

[tool.ruff]
line-length = 120
ignore = ["E731"]
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 this rule to ignore lambda assignment (https://beta.ruff.rs/docs/rules/lambda-assignment/) as we have these everywhere and imo they're ok as long as you don't have a ton of logic in them.

@waydegg
Copy link
Contributor Author

waydegg commented Aug 30, 2023

In an effort to keep the number of changes down in this PR, I think we should add fuzzy matching in a separate feature branch. Same goes for the @ directive since it's basically another completer we're adding (e.g. another feature).

@waydegg
Copy link
Contributor Author

waydegg commented Aug 30, 2023

I don't think it's worth the effort to generate completions from ctags. You're only able to get top-level syntax words (can't get anything inside classes/functions), so the pygments lexer does a pretty good job at this as is.

I also decided to not include completions from all files in the git repo (if there is one), only files explicitly added in the code context, since there's too much irrelevant information being suggested. This is the how VSCode does their compeltions anyways which I think is a good choice.

@waydegg waydegg marked this pull request as ready for review August 30, 2023 19:54
@granawkins
Copy link
Member

I read through this and tested it out, and it looks good to me. One comment: eventually it'd be nice if this works with different interfaces. Like if I could call refresh_completions from the VSCode extension somehow with its current user input. This can def be done later, just wanted to throw it out there.

@biobootloader
Copy link
Member

I read through this and tested it out, and it looks good to me. One comment: eventually it'd be nice if this works with different interfaces. Like if I could call refresh_completions from the VSCode extension somehow with its current user input. This can def be done later, just wanted to throw it out there.

Agreed, "core" Mentat would provide lists of available completions to all interfaces

@biobootloader
Copy link
Member

In an effort to keep the number of changes down in this PR, I think we should add fuzzy matching in a separate feature branch. Same goes for the @ directive since it's basically another completer we're adding (e.g. another feature).

Agreed, let's make followup issues for these. The @ approach also relates to #17

@waydegg
Copy link
Contributor Author

waydegg commented Aug 31, 2023

One comment: eventually it'd be nice if this works with different interfaces

Totally. I'd like to have the completions using pygls to pull from language servers (assuming we can get it working outside of VSCode). Would make the auto-completions tool-agnostic and higher quality.

@biobootloader
Copy link
Member

not a big deal because it's rare, but both kinds of completitons can be suggested at once (the history one take priority if you hit tab):
image

I don't think it's worth trying to fix - not sure how often most people would use history completions anyway

@waydegg
Copy link
Contributor Author

waydegg commented Sep 1, 2023

both kinds of completitons can be suggested at once

Do you have any preference on how we should handle this? It's pretty common for me to see both completion sources, so I think it's worth looking into.

I'm thinking we could copy how ipython does it and have arrow keys finish/cycle command history completions, and have tab/shift+tab or ctrl+n/ctrl+p finish/cycle auto-completions.

Comment on lines 124 to 81
if self.files:
if self.code_context.files:
cprint("Files included in context:", "green")
else:
cprint("No files included in context.\n", "red")
cprint("Git project: ", "green", end="")
cprint(self.git_root.name, "blue")
cprint(self.config.git_root.name, "blue")
_print_path_tree(
_build_path_tree(self.files, self.git_root),
get_paths_with_git_diffs(self.git_root),
self.git_root,
_build_path_tree(self.code_context.files.values(), self.config.git_root),
get_paths_with_git_diffs(self.config.git_root),
self.config.git_root,
)
print()
Copy link
Member

Choose a reason for hiding this comment

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

I think displaying the project tree / include files should be done by CodeContext. And instead of being in the init should be in something like CodeContext.display_context_tree(), called in app.py after initializing the CodeContext.

@biobootloader
Copy link
Member

I'm thinking we could copy how ipython does it and have arrow keys finish/cycle command history completions, and have tab/shift+tab or ctrl+n/ctrl+p finish/cycle auto-completions.

That sounds great!

Copy link
Member

@biobootloader biobootloader left a comment

Choose a reason for hiding this comment

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

This is awesome!

@biobootloader biobootloader merged commit e99ed70 into AbanteAI:main Sep 5, 2023
8 checks passed
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.

4 participants