Skip to content

Add support for running inference against the org endpoint #66

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

Merged
merged 4 commits into from
Jun 18, 2025

Conversation

cschleiden
Copy link
Member

This pull request introduces support for specifying an organization when running evaluations or inference against models, along with refactoring the Azure client configuration for improved flexibility. Key changes include adding the --org flag to relevant commands, updating the Azure client to dynamically construct inference URLs based on the organization, and modifying associated tests to accommodate the new functionality.

@cschleiden cschleiden force-pushed the garman/add-org-support branch from 3096d1c to 68ce62d Compare June 17, 2025 09:15
@cschleiden cschleiden marked this pull request as ready for review June 17, 2025 09:16
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 09:16
@cschleiden cschleiden requested a review from a team as a code owner June 17, 2025 09:16
Copy link
Contributor

@Copilot 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 enables specifying an organization when running model inference by adding an --org flag, refactors Azure client configuration to separate root and path, and updates tests to accommodate the new method signatures.

  • Introduce --org flag in run and eval commands and propagate it to the client
  • Refactor AzureClientConfig to use InferenceRoot + InferencePath and update GetChatCompletionStream to build URLs with an optional org segment
  • Update mocks, interfaces, and tests for the new org parameter

Reviewed Changes

Copilot reviewed 36 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/azuremodels/unauthenticated_client.go Added org parameter to stubbed streaming method
internal/azuremodels/types.go Added Organization field to ChatCompletionOptions
internal/azuremodels/mock_client.go Updated mock signature and implementation to include org
internal/azuremodels/client.go Updated interface to accept org argument
internal/azuremodels/azure_client_test.go Switched tests to use InferenceRoot and updated calls
internal/azuremodels/azure_client_config.go Split default inference URL into root and path fields
internal/azuremodels/azure_client.go Implemented org-aware URL construction for inference calls
cmd/run/run.go Added --org flag, examples, and passed org to handler
cmd/run/run_test.go Updated run command tests to expect org parameter
cmd/eval/eval.go Added --org flag, examples, and passed org to handler
cmd/eval/eval_test.go Updated eval tests to expect org parameter
Comments suppressed due to low confidence (4)

cmd/run/run.go:416

  • The help text for the --org flag is missing a closing parenthesis. Consider updating it to: "Organization to attribute usage to (omitting will attribute usage to the current actor)".
cmd.Flags().String("org", "", "Organization to attribute usage to (omitting will attribute usage to the current actor")

cmd/eval/eval.go:134

  • The help text for the --org flag is missing a closing parenthesis. Consider updating it to: "Organization to attribute usage to (omitting will attribute usage to the current actor)".
cmd.Flags().String("org", "", "Organization to attribute usage to (omitting will attribute usage to the current actor")

internal/azuremodels/azure_client_test.go:52

  • In tests, InferencePath remains unset and defaults to empty, relying on the server handling /. For clarity and future compatibility, explicitly set InferencePath: defaultInferencePath in the test config.
cfg := &AzureClientConfig{InferenceRoot: testServer.URL}

internal/azuremodels/client.go:7

  • The doc comment for GetChatCompletionStream should be updated to reflect the new org parameter in the method signature.
// GetChatCompletionStream returns a stream of chat completions using the given options.

@cschleiden cschleiden force-pushed the garman/add-org-support branch from 3afdc58 to a787251 Compare June 17, 2025 10:02
@cschleiden cschleiden merged commit 9414f17 into main Jun 18, 2025
5 checks passed
@cschleiden cschleiden deleted the garman/add-org-support branch June 18, 2025 08:48
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.

3 participants