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

feat: agent enable per request prefix/suffix max lines configuration #255

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

ghthor
Copy link
Contributor

@ghthor ghthor commented Jun 22, 2023

What Changed

  1. bug: fix vim agent startup race condition
  2. feat: add per request max lines config for prefix/suffix to vscode/vim
    clients

Why

1. bug: vim agent startup race condition

At some point the nodejs tabby-agent was updated, but the compiled bundle for
the vim client was not updated. Once updating there was a race condition where
the initialize call from the vim client was failing because the agent hadn't
been bound to the StdIO object BEFORE StdIO started processing requests from
stdin.

2. feat: add per request max lines config for prefix/suffix to vscode/vim

I initially wanted to configure this amount to test if it had any effect on the
quality of the completions. I think this will be useful to test on different
models and making it easy to change without requiring a recompile of the
tabby-agent seems very useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the race condition causing the vim client to fail to initialize the agent

Copy link
Member

Choose a reason for hiding this comment

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

nice catch! cc @icycodes

Copy link
Member

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, @ghthor.

Overall, it looks good to me. However, I would like to bring your attention to a scheduled refactor for the developer options:

  1. All options under developerOptions will no longer be configurable in IDE/Editor extensions. For example, developerOptions.suggestionDelay will be removed from the VSCode settings.

  2. The Tabby agent will attempt to read these developer options from ~/.tabby/agent/config.toml.

By implementing these changes, we aim to reduce the maintenance effort required for each IDE/Editor extension. Only the ability to enable or disable the options will be exposed.

Copy link
Member

Choose a reason for hiding this comment

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

nice catch! cc @icycodes

@wsxiaoys wsxiaoys requested a review from icycodes June 22, 2023 01:53
Copy link
Member

@icycodes icycodes left a comment

Choose a reason for hiding this comment

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

LGTM

@wsxiaoys wsxiaoys merged commit 450e6bb into TabbyML:main Jun 22, 2023
@ghthor
Copy link
Contributor Author

ghthor commented Jun 22, 2023

The Tabby agent will attempt to read these developer options from ~/.tabby/agent/config.toml.

I don't see any issue with that. I'd suggest adding a command we can send to the tabby-agent in addition to support for handling the SIGHUP signal that would trigger tabby-agent to re-read from this configuration.

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.

None yet

3 participants