Use builtin OpenAI plugin. Adds tool support. #18

kanaka · 2025-06-12T00:02:06Z

The GitHub Copilot completions (/chat/completions) API is based on the OpenAI API regardless of whether the backend model is an OpenAI one or not. This means we can greatly simplify the GitHub Copilot plugin by using the builtin OpenAI plugin (llm.default_plugins.openai_models) for everything except for the Copilot device-based authentication.

In addition to the significant simplification and code reduction, one key benefit of this approach is that we automatically get additional functionality that is implemented by the openai_models plugin such as tool calling, async, schema/structured output, etc. We should also get any future OpenAI plugin functionality automatically (or with minimal changes).

Tests have been updated to remove most of the completion tests since this should now be covered by tests in OpenAI plugin itself. Minor updates were needed to the authentication and the CLI tests to get them to work simplified code base.

Finally, the fetch_models_data top-level function (wraps _fetch_models_data) has been memoized so that the model data is cached after the first call.

NOTES:

To support tool calls in non OpenAI models, this change is needed in llm itself: Handle None in tool/function arguments. simonw/llm#1170 However, this change isn't dependent on that one (but once it's merge tool calls will work in anthropic models too).
I haven't bumped the version as part of this PR but that should happen before this is released to pypi of course.
I removed the use of "github_copilot" as a default model. Trying to keep it was increasing the complexity of the result quite a bit. However, I don't feel strongly about that if somebody else wants to add it back.

kanaka · 2025-06-12T15:55:40Z

I suspect the prior failures were due to the tests picking up my real auth'd model data (because they worked for me locally). I updated the three failing tests to inject mocked model data directly.

kanaka · 2025-06-13T01:32:04Z

@jmdaly can you kick off the workflow again to see if my changes fixed the 3 test failures?

jmdaly · 2025-06-13T13:25:38Z

@jmdaly can you kick off the workflow again to see if my changes fixed the 3 test failures?

Done!

kanaka · 2025-06-13T16:52:50Z

@jmdaly Ah, apparently my existing auth config was allowing those three tests to pass. I'm pretty confident I've fixed those test issues now (an additional monkey patch in the auth path). I've also added a Dockerfile and test script that makes it easier to test in an uncontaminated environment. The script run-tests.sh does the same linting and test steps in the testing workflow (could be used to replace a couple of those workflow steps if you want).

Could you kick off the workflow again?

kanaka · 2025-06-13T16:59:57Z

@jmdaly You might be interested in this post where I leverage llm and this branch of llm-github-copilot to show how to build your own coding agent: https://kanaka.github.io/blog/llm-agent-in-five-steps/

jmdaly · 2025-06-14T19:59:16Z

That's an awesome blog post!! Thanks for sharing :) I'm still hoping to review this PR and get it merged as soon as I can!

The GitHub Copilot completions (`/chat/completions`) API is based on the OpenAI API regardless of whether the backend model is an OpenAI one or not. This means we can greatly simplify the GitHub Copilot plugin by using the builtin OpenAI plugin (`llm.default_plugins.openai_models`) for everything except for the Copilot device-based authentication. In addition to the significant simplification and code reduction, one key benefit of this approach is that we automatically get additional functionality that is implemented by the openai_models plugin such as tool calling, async, schema/structured output, etc. We should also get any future OpenAI plugin functionality automatically (or with minimal changes). Tests have been updated to remove most of the completion tests since this should now be covered by tests in OpenAI plugin itself. Minor updates were needed to the authentication and the CLI tests to get them to work simplified code base. Finally, the fetch_models_data top-level function (wraps _fetch_models_data) has been memoized so that the model data is cached after the first call.

Make sure the model data comes from the mocked data and not accidentally from an actual connection to GitHub Copilot. This updates these tests: test_prompt, test_model_variants, test_options.

Add a Dockerfile that packages up the code and tests so that they can be executed in an isolated environment. This avoids environmental contamination such as auth config/keys may impact certain code paths succeed when they otherwise shouldn't. Example usage: docker build -t llm-github-copilot-test . docker run -it llm-github-copilot-test

The _GithubCopilot class was inheriting from openai_models.Chat. The derived classes are where the actual inheritance/mixin should happen. The extra inherit seemed to be harmless for now but could cause problems in the future since it could mix Chat and AsyncChat functionality by accident.

kanaka · 2025-06-16T14:53:54Z

@jmdaly I rebased to address the merge conflicts from the other PR merge. I also just pushed a tiny cleanup to the class inheritance that I discovered.

The resulting diff is a little bit hard to read. Here's the tl;dr version:

GitHubCopilotAuthenticator class is unchanged.
Replace GithubCopilot class with sync and async classes that wrap the builtin openai classes and that just override auth key, copilot api_base, and capabilities info
Simplify how model data is fetched (and add caching/memoizing to it)
Drop the default model handling conditions since it wasn't immediately clear how this should integrate with openai builtin classes.

jmdaly · 2025-06-17T22:51:21Z

This looks good to me! @cavanaug you've done a fair amount of work in this repo - do these changes look good to you?

jmdaly · 2025-06-17T22:52:04Z

Also, removing almost 800 lines is awesome :)

cavanaug · 2025-06-17T23:26:49Z

Let me look into this in general and test it a bit, but I really do like the concept of reusing an existing library and automatically gaining benefits. Though for tool calling we will want to stay in alignment with whatever Simon does with llm in general, we dont want to expose any additional potential security implications with just this plugin.

kanaka · 2025-06-18T16:31:20Z

@cavanaug Hopefully this should make it a lot easier to stay in sync with functionality in llm (including tools), because with this change, the plugin now only does auth(

llm-github-copilot/llm_github_copilot.py

Lines 476 to 509 in d9e1202

    
           def __init__(self, model_id, model_data, *args, **kwargs): 
        
               # Get API key 
        
               self.authenticator = GitHubCopilotAuthenticator() 
        
               try: 
        
                   api_key = self.authenticator.get_api_key() 
        
               except Exception as e: 
        
                   error_message = str(e) 
        
                   if "authentication required" in error_message.lower(): 
        
                       print("GitHub Copilot authentication required. Run 'llm github_copilot auth login' to authenticate.") 
        
                   else: 
        
                       print(f"Error getting GitHub Copilot API key: {error_message}") 
        
                   raise 
        
               headers = self.authenticator._get_github_headers() 
        
               if "/" in model_id: 
        
                   _, model_name = model_id.split("/", 1) 
        
               else: 
        
                   model_name = model_id 
        
               supports = model_data.get("capabilities", {}).get("supports", {}) 
        
               super().__init__( 
        
                       model_id, 
        
                       *args, 
        
                       key=api_key, 
        
                       model_name=model_name, 
        
                       api_base=self.API_BASE, 
        
                       headers=headers, 
        
                       can_stream=supports.get("streaming", False), 
        
                       supports_tools=supports.get("tool_calls", False), 
        
                       supports_schema=supports.get("structured_outputs", False), 
        
                       **kwargs) 
        
           def __str__(self): 
        
               return "GitHub Copilot Chat: {}".format(self.model_id)

) and model querying. All the other API calls and functionality are handled by the builtin openai plugin (that Simon maintains in llm). Any tool policy or security issues would be handled in the builtin openai plugin and be transparent to the llm-github-copilot plugin. Thinking pretty far out of the box, there is a weird case where a tool is trying to query auth settings, but I would expect that to just break things rather than expose a vulnerability because the plugin doesn't do anything except set the api url and auth key during __init__.

cavanaug · 2025-06-19T09:20:45Z

I spent some time looking at this and testing it locally. LGTM. Great simplification!!

jmdaly · 2025-06-20T12:42:11Z

Great! Thanks @kanaka for the great PR and @cavanaug for reviewing it! Let's merge it :)

kanaka · 2025-06-20T15:53:53Z

Yay! @jmdaly when you have a chance can you update the version and push a release? Thanks for your work on this plugin!

jmdaly · 2025-06-21T01:39:24Z

Yay! @jmdaly when you have a chance can you update the version and push a release? Thanks for your work on this plugin!

Yes definitely! I'll try to get it done this weekend :) Thanks again for your great contribution!

jmdaly · 2025-06-24T01:21:01Z

Hi @kanaka, just wanted to let you know that version 0.3 with your improvements has been released to PyPi :)

This was referenced Jun 12, 2025

Handle None in tool/function arguments. simonw/llm#1170

Closed

llm tool calling support #12

Closed

kanaka added 3 commits June 16, 2025 13:59

tests: add mock model data to 3 tests.

4f5d84e

Make sure the model data comes from the mocked data and not accidentally from an actual connection to GitHub Copilot. This updates these tests: test_prompt, test_model_variants, test_options.

kanaka force-pushed the main branch from 1119e6e to 0824e55 Compare June 16, 2025 14:11

jmdaly merged commit 3230e6f into jmdaly:main Jun 20, 2025
5 checks passed

Use builtin OpenAI plugin. Adds tool support. #18

Use builtin OpenAI plugin. Adds tool support. #18

Uh oh!

Conversation

kanaka commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kanaka commented Jun 12, 2025

Uh oh!

kanaka commented Jun 13, 2025

Uh oh!

jmdaly commented Jun 13, 2025

Uh oh!

kanaka commented Jun 13, 2025

Uh oh!

kanaka commented Jun 13, 2025

Uh oh!

jmdaly commented Jun 14, 2025

Uh oh!

kanaka commented Jun 16, 2025

Uh oh!

jmdaly commented Jun 17, 2025

Uh oh!

jmdaly commented Jun 17, 2025

Uh oh!

cavanaug commented Jun 17, 2025

Uh oh!

kanaka commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cavanaug commented Jun 19, 2025

Uh oh!

jmdaly commented Jun 20, 2025

Uh oh!

Uh oh!

kanaka commented Jun 20, 2025

Uh oh!

jmdaly commented Jun 21, 2025

Uh oh!

jmdaly commented Jun 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kanaka commented Jun 12, 2025 •

edited

Loading

kanaka commented Jun 18, 2025 •

edited

Loading