Skip to content

Update inference endpoint in models client to use the now preferred models.github.ai#42

Merged
garman merged 1 commit intogithub:mainfrom
garman:change-backing-service
Apr 25, 2025
Merged

Update inference endpoint in models client to use the now preferred models.github.ai#42
garman merged 1 commit intogithub:mainfrom
garman:change-backing-service

Conversation

@garman
Copy link
Copy Markdown
Contributor

@garman garman commented Apr 22, 2025

Aims to do the following

  • update the base url to point to models.github.ai
  • update the model param formatting to {publisher}/{name}
  • update relevant documentation
  • update display of list view
  • update how the models are displayed from a bare run or view command
  • add a FormatIdentifier func for consistently displaying model ID's in the places we do so

GitHub models is switching from using https://models.inference.ai.azure.com/chat/completions as the target endpoint for inference. The preferred inference endpoint is now https://models.github.ai/inference/chat/completions. models.github.ai expects the model param to be a composite of the model's publisher and the model's name -- downcased and with - in pace of .

The url and required param format changes made the changes to some verbiage and how we compare and display model names (now called ID in the list output) a requirement as well.


NOTE: some naming of packages, files, and functions may no longer be fully accurate. Updating that naming is out of scope for this change, and may still be inconsistent even if updated given the current transition period.

* updates the base url
* updates relevant documentation
* updates display of list view
* updates how the models are displayed from a bare `run` or `view` command
* adds a FormatIdentifier func for consistently displaying model ID's in the places we do so

GitHub models is switching from using `https://models.inference.ai.azure.com/chat/completions` as the target
endpoint for inference. The suggested inference endpoint is now
`https://models.github.ai/inference/chat/completions`. `models.github.ai` expects the model param to be a composite
of the model's publisher and the model's name -- downcased and with `-` in pace of ` `.

The url change, and required param format change, made the changes to some verbiage and how we compare and display
model names (now called `ID` in the `list` output) a requirement as well.
Copilot AI review requested due to automatic review settings April 22, 2025 01:13
@garman garman requested a review from a team as a code owner April 22, 2025 01:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the inference endpoint and changes the model parameter formatting to use a composite identifier (publisher/name) throughout the client, tests, and documentation.

  • Update default endpoint URL from Azure to GitHub models
  • Replace bare model names with composite identifiers via FormatIdentifier
  • Adjust tests, CLI commands, and docs accordingly

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/azuremodels/model_summary_test.go Updated tests to validate composite identifier behavior
internal/azuremodels/model_summary.go Modified HasName and sorting logic to use composite identifiers
internal/azuremodels/model_details_test.go Added tests for the new FormatIdentifier function
internal/azuremodels/model_details.go Implemented FormatIdentifier to format publisher and model names
internal/azuremodels/azure_client_config.go Updated default inference endpoint URL
cmd/view/view_test.go Updated tests to use composite identifiers for the view command
cmd/view/view.go Revised examples and prompt options to use composite identifiers
cmd/run/run_test.go Updated tests to use composite identifiers for the run command
cmd/run/run.go Updated prompt options and removed legacy bare model name handling
cmd/root.go Minor import reordering
cmd/list/list_test.go Updated header and tests to reflect composite identifier usage
cmd/list/list.go Changed table display to show the composite identifier instead of bare name
README.md Updated CLI usage examples and instructions with composite identifier
Comments suppressed due to low confidence (2)

cmd/run/run.go:441

  • The removal of reassigning modelName to the bare model name was intentional to enforce the composite identifier format. Please verify that downstream logic correctly handles the composite identifier.
modelName = model.Name

internal/azuremodels/model_summary.go:28

  • HasName now only compares against the composite identifier, removing the fallback to FriendlyName or bare Name. Please confirm that this change in behavior aligns with the updated requirements.
modelID := FormatIdentifier(m.Publisher, m.Name)

Comment on lines +32 to +33
// Replace spaces with dashes and convert to lowercase
result := strings.ToLower(s)
Copy link

Copilot AI Apr 22, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider trimming whitespace from the publisher and model name before formatting in FormatIdentifier to avoid unintended hyphens from extra spaces.

Suggested change
// Replace spaces with dashes and convert to lowercase
result := strings.ToLower(s)
// Trim whitespace, replace spaces with dashes, and convert to lowercase
result := strings.TrimSpace(s)
result = strings.ToLower(result)

Copilot uses AI. Check for mistakes.
Comment thread README.md
```

Use the value in the "Name" column when specifying the model on the command-line.
Use the value in the "ID" column when specifying the model on the command-line.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels more expected to pass some 'id' field in a command, instead of 'name'. 👍🏻

@garman garman merged commit 290f4d6 into github:main Apr 25, 2025
2 checks passed
@garman garman deleted the change-backing-service branch April 25, 2025 13:24
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