Fixes #250 - All SDKs Validate Session Config Model#378
Fixes #250 - All SDKs Validate Session Config Model#378john-mckillip wants to merge 12 commits intogithub:mainfrom
Conversation
Validate that SessionConfig.Model is a valid model name before creating a session. When an invalid model is specified, throw an ArgumentException with the invalid model name and list of available models. Fixes github#250
There was a problem hiding this comment.
Pull request overview
This PR adds model validation to the .NET SDK's CreateSessionAsync method to ensure that only valid model IDs are used when creating sessions. The implementation throws an ArgumentException with a helpful error message listing available models when an invalid model is specified.
Changes:
- Added validation logic in
Client.csto verify the model ID exists in the list of available models before creating a session - Updated existing test to use a valid model name (
claude-sonnet-4.5) instead of a placeholder - Added new test to verify that invalid model names trigger an
ArgumentExceptionwith appropriate error details
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/src/Client.cs | Implements model validation by checking the provided model against available models and throwing ArgumentException if invalid |
| dotnet/test/SessionTests.cs | Updates test with valid model name and adds new test case for invalid model validation |
…larifying the minimal performance overhead
Validate that the model specified in SessionConfig exists in the list of available models before creating a session. Raises ValueError with the invalid model name and available alternatives. Uses list_models() which caches results after the first call, so validation adds minimal overhead. Fixes github#250
Validate that the specified model exists in the available models list when creating a new session. This brings the Go SDK in line with the Python and .NET implementations. Changes: - Add model validation in Client.CreateSession that checks against ListModels output using case-insensitive comparison - Return descriptive error listing available models on validation failure - Add TestClient_CreateSession_WithInvalidModel unit test - Refactor to use strings.ToLower and strings.Join directly for performance - Reuse cached model list from ListModels to minimize overhead The validation ensures users receive immediate feedback if they specify an invalid model name, rather than encountering errors later in the session lifecycle.
Validate that the specified model exists in the available models list when creating a new session. This brings the Node.js SDK in line with the Python, .NET, and Go implementations. Changes: - Add model validation in CopilotClient.createSession that checks against listModels output using case-insensitive comparison - Throw descriptive error listing available models on validation failure - Add unit test "throws error when creating session with invalid model" in client.test.ts - Optimize performance using Array.some() for short-circuit search and single lowercase conversion of input model - Reuse cached model list from listModels() to minimize overhead The validation ensures users receive immediate feedback if they specify an invalid model name, rather than encountering errors later in the session lifecycle.
john-mckillip
changed the title
Replace HashSet-based model validation with a more idiomatic LINQ Any() expression using case-insensitive string comparison. This improves readability and aligns with other SDK implementations. - Remove intermediate HashSet creation and validModelIds variable - Use string.Equals with StringComparison.OrdinalIgnoreCase for consistency - Update error message to use availableModels.Select(m => m.Id) inline
| { | ||
| throw new ArgumentException( | ||
| $"Invalid model '{config.Model}'. Available models: {string.Join(", ", availableModels.Select(m => m.Id))}", | ||
| nameof(config)); |
There was a problem hiding this comment.
The nameof(config) should be nameof(config.Model) since the validation is specifically for the Model property, not the entire config object. This provides more precise error information to the caller.
| nameof(config)); | |
| nameof(config.Model)); |
There was a problem hiding this comment.
Thanks for the review! However, I need to push back on this suggestion. The ArgumentException
constructor's paramName parameter should reference the actual method parameter name, not a
property of that parameter.
According to Microsoft's ArgumentException documentation,
the paramName should be "the name of the parameter that caused the current exception."
In this case:
- The method parameter is
config(of typeSessionConfig?) - There is no parameter named
Modelin the method signature - Using
nameof(config.Model)causes SonarQube warning: "The parameter name 'Model' is not
declared in the argument list"
The error message itself already clearly indicates which property is invalid:
"Invalid model '{config.Model}'. Available models: ..."
This is consistent with standard .NET practices where you reference the parameter, and the
error message provides the specific details about what's wrong with it.
…nAsync in the dotnet SDK client.
1 participant
This pull request introduces model validation when creating a session in the Copilot SDK clients across all supported languages (Node.js, Python, Go, and .NET). Now, if a user attempts to create a session with a model name that does not exist, a clear and descriptive error is thrown, listing available models. Additionally, comprehensive tests have been added to ensure this validation works as intended. Some test cases have also been updated to use a real model name instead of a placeholder.
Model validation and error handling improvements:
nodejs,python,go,dotnet) now validate themodelparameter increateSession/CreateSessionAsync/CreateSessionmethods. If an invalid model is specified, an informative error is thrown, listing available models. [1] [2] [3] [4]Testing enhancements:
Test updates for valid model usage:
"claude-sonnet-4.5"instead of the placeholder"fake-test-model". [1] [2] [3] [4] [5]Utility improvements (Go SDK):
containsfor case-insensitive substring checks in Go tests.These changes improve the robustness and user experience of the Copilot SDKs by ensuring that only valid models can be used when creating sessions and by providing clear feedback when an invalid model is specified.