Expose templates for customisation in providers

[krassowski]

References

• Fixes Allow prompt template customization for Jupyter AI chat.  #498
• Closes [WIP] Configurable prompt templates #449

Code changes

Adds:

• adds get_chat_prompt_template() to BaseProvider, allowing custom providers to override chat prompt template.
• adds get_inline_completion_prompt_template() for completion template

[Marxlp]

Nice work. Besides, I think it would be better to be able to configure prompt on the UI.

[krassowski]

Besides, I think it would be better to be able to configure prompt on the UI.

I think this could be implemented in a follow up PR; the UI template should probably take precedence over provider-defined template.

[dlqqq]

Rebased this on latest main, reviewing & testing now.

[dlqqq]

Tested this locally and everything works. My only concern is regarding the method name; I would prefer get_completion_prompt_template() and drop the inline word. We should keep inline implicit if we don't support any other types of completion.

Also, I foresee difficulty with backporting this PR to 1.x as-is, since it also includes changes to the autocomplete work that is exclusive to 2.x. It might be easier to split the autocomplete-related changes into a separate PR and make this one smaller and "more backport-able". However, this is up to you, and I leave you the responsibility of backporting this PR. 👍

Finally, can you also consider a follow-up PR to remove the word inline from the completion handler classes? For example, DefaultInlineCompletionHandler => DefaultCompletionHandler. I would greatly prefer shorter and less redundant names.

[krassowski]

We should keep inline implicit if we don't support any other types of completion.

Well, this is where we have different opinions - in #465 I included a detailed reasoning for using "inline completion" over just "completion" or other terms, let me quote it here:

• why use "inline completion" vs "infill"? For consistency with other editors/frontend/LSP I went for "inline completions"; while this is a presentation detail, it does define what kind of information should be returned from models (a long multi-line suggestion; as compared to completions for tab-completions which would be also a kind of infill but needing many short, single-line completions)
• why api/ai/completions/inline? to allow for creation of non-inline completion endpoints in the future

If you a specific term was used it would be easier in future to introduce other completion types. For example, some users prefer a AI-powered tab-completer which uses learned patterns to infer top k tokens to prioiritise (see tabnine, previously kite). As far as I know this is also the code name used in other IDEs so keeping it explicit makes the codebase easy to navigate for others.

Also, I foresee difficulty with backporting this PR to 1.x as-is, since it also includes changes to the autocomplete work that is exclusive to 2.x. It might be easier to split the autocomplete-related changes into a separate PR and make this one smaller and "more backport-able". However, this is up to you, and I leave you the responsibility of backporting this PR. 👍

Yes, I will take care of backport this - no worries.

[krassowski]

Finally, can you also consider a follow-up PR to remove the word inline from the completion handler classes? For example, DefaultInlineCompletionHandler => DefaultCompletionHandler. I would greatly prefer shorter and less redundant names.

Sure if I have not convinced you on this one in the message above, I will open such a PR - just let me know :)

I would prefer get_completion_prompt_template() and drop the inline word.

Done in b235e0f.

[krassowski]

Rebased to pickup a single-word change in README 😆

[dlqqq]

@krassowski Thanks for addressing my feedback. I've approved this PR; you should have permission to merge & backport this PR yourself. If not, just ping me. 👍

Regarding "inline completion" v.s. "completion": I still prefer "completion" to be internally consistent; I think it's awkward how we only include the word "inline" when we have enough space for it. But I won't block on this, since you certainly know more about this topic than I do.

[krassowski]

@meeseeksdev please backport to 1.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 1.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 9907cc62b79738c4025c5949a576d8954613a67a

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #581: Expose templates for customisation in providers'

4. Push to a named branch:

git push YOURFORK 1.x:auto-backport-of-pr-581-on-1.x

5. Create a PR against branch 1.x, I would have named this PR:

"Backport PR #581 on branch 1.x (Expose templates for customisation in providers)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.