feat:Enhance LangSmith SSO settings and code evaluation with new classes and APIs#48
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
WalkthroughThe changes introduce several new classes and properties to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrgsClient
participant API
participant SSOProvider
Client->>OrgsClient: Create SSO Settings
OrgsClient->>API: POST /api/v1/orgs/current/sso-settings
API-->>OrgsClient: Response with SSO Settings
OrgsClient-->>Client: Return SSO Settings
Client->>OrgsClient: Get Current SSO Settings
OrgsClient->>API: GET /api/v1/orgs/current/sso-settings
API-->>OrgsClient: Response with SSO Providers
OrgsClient-->>Client: Return SSO Providers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (7)
src/libs/LangSmith/Generated/LangSmith.Models.SSOSettingsCreateAttributeMapping.g.cs (1)
6-8: Suggestion: Add a descriptive summary.The summary documentation is currently empty. It would be beneficial to provide a brief description of the
SSOSettingsCreateAttributeMappingclass to improve code readability and maintainability.src/libs/LangSmith/Generated/LangSmith.Models.TriggerRulesApiV1RunsRulesTriggerPostResponse.g.cs (1)
6-8: Suggestion: Add a descriptive summary.The class summary is empty. Adding a descriptive summary would enhance the understanding of the class's purpose and usage.
src/libs/LangSmith/Generated/LangSmith.Models.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse.g.cs (1)
6-8: Suggestion: Add a descriptive summary.It is recommended to add a descriptive summary to the class documentation to clarify its role and functionality.
src/libs/LangSmith/Generated/LangSmith.Models.CodeEvaluatorTopLevel.g.cs (1)
6-23: Well-structured class for JSON serialization.The
CodeEvaluatorTopLevelclass is well-defined with appropriate JSON serialization attributes and nullable reference types. The use ofrequiredkeyword andJsonRequiredattribute ensures strict data handling, which is crucial for the integrity of the data model.However, the XML documentation comments are missing descriptions. Adding detailed comments would improve code maintainability and readability.
Consider adding detailed XML documentation comments to explain the purpose and usage of the class and its properties.
src/libs/LangSmith/Generated/LangSmith.Models.TriggerRulesRequest.g.cs (1)
6-23: Well-structured class for JSON serialization.The
TriggerRulesRequestclass is well-defined with appropriate JSON serialization attributes and nullable reference types. The use ofrequiredkeyword andJsonRequiredattribute ensures strict data handling, which is crucial for the integrity of the data model.However, the XML documentation comments are missing descriptions. Adding detailed comments would improve code maintainability and readability.
Consider adding detailed XML documentation comments to explain the purpose and usage of the class and its properties.
src/libs/LangSmith/Generated/LangSmith.Models.SSOSettingsCreate.g.cs (1)
6-37: Well-structured class with flexible JSON handling.The
SSOSettingsCreateclass is well-defined with appropriate JSON serialization attributes, custom converters, and nullable reference types. The use of custom JSON converters (AnyOfJsonConverterFactory2) forMetadataXmlandMetadataUrlis particularly noteworthy for handling different data types flexibly.However, the XML documentation comments are missing descriptions, and there is a pragma directive to disable warnings for obsolete members. Adding detailed comments would improve code maintainability and readability, and the use of obsolete members should be reviewed to ensure they are still necessary.
Consider adding detailed XML documentation comments to explain the purpose and usage of the class and its properties. Review the necessity of the pragma directive for obsolete members.
src/libs/LangSmith/Generated/LangSmith.Models.RunRulesSchema.g.cs (1)
157-163: Approve the addition ofCodeEvaluatorsbut recommend adding a descriptive summary.The new property
CodeEvaluatorsis correctly implemented with appropriate JSON serialization attributes and uses theSystem.AnyOftype for flexibility. However, the summary tag is empty and should be filled with a descriptive comment to explain the purpose and usage of this property.Consider adding a descriptive summary to the property:
- /// <summary> - /// - /// </summary> + /// <summary> + /// Holds a collection of code evaluator configurations that can be used to dynamically assess code rules. + /// </summary>
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- src/libs/LangSmith/Generated/LangSmith.Models.CodeEvaluatorTopLevel.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.OrgFeatureFlags.g.cs (3 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.OrganizationConfig.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.RunRulesCreateSchema.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.RunRulesSchema.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.SSOProvider.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.SSOSettingsCreate.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.SSOSettingsCreateAttributeMapping.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.TriggerRulesApiV1RunsRulesTriggerPostResponse.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.TriggerRulesRequest.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.OrgsClient.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPost.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.OrgsClient.GetCurrentSsoSettingsApiV1OrgsCurrentSsoSettingsGet.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.RunClient.CreateRuleApiV1RunsRulesPost.g.cs (3 hunks)
- src/libs/LangSmith/Generated/LangSmith.RunClient.TriggerRulesApiV1RunsRulesTriggerPost.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.RunClient.UpdateRuleApiV1RunsRulesRuleIdPatch.g.cs (3 hunks)
- src/libs/LangSmith/openapi.yaml (10 hunks)
Additional comments not posted (19)
src/libs/LangSmith/Generated/LangSmith.Models.SSOSettingsCreateAttributeMapping.g.cs (2)
2-2: Approved: Use of nullable reference types.The use of
#nullable enableis appropriate and enhances type safety by explicitly handling null values in the code.
12-16: Approved: Use of JsonExtensionData attribute.The use of
JsonExtensionDataattribute for theAdditionalPropertiesdictionary is appropriate. It allows for serialization and deserialization of additional JSON properties that are not matched by the class properties, which is useful for handling dynamic properties in JSON data.src/libs/LangSmith/Generated/LangSmith.Models.TriggerRulesApiV1RunsRulesTriggerPostResponse.g.cs (2)
2-2: Approved: Use of nullable reference types.Enabling nullable reference types is a good practice for modern C# development, ensuring better handling of null values.
12-16: Approved: Use of JsonExtensionData attribute.The
JsonExtensionDataattribute is correctly applied to theAdditionalPropertiesdictionary, facilitating the handling of extra JSON properties that do not have corresponding class properties.src/libs/LangSmith/Generated/LangSmith.Models.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse.g.cs (2)
2-2: Approved: Use of nullable reference types.Continued use of nullable reference types across new files is a good practice, enhancing type safety and null handling in the codebase.
12-16: Approved: Use of JsonExtensionData attribute.The application of the
JsonExtensionDataattribute on theAdditionalPropertiesdictionary is correct and useful for handling additional JSON properties dynamically.src/libs/LangSmith/Generated/LangSmith.Models.OrganizationConfig.g.cs (1)
82-86: Well-implemented new propertyCanUseSamlSso.The addition of the
CanUseSamlSsoproperty is well-executed with clear documentation and appropriate default settings:
- The property is correctly annotated for serialization.
- The default value of
falseis a prudent choice for security-sensitive settings.This change enhances the configuration capabilities of the
OrganizationConfigclass by allowing it to manage SAML SSO settings effectively.src/libs/LangSmith/Generated/LangSmith.OrgsClient.GetCurrentSsoSettingsApiV1OrgsCurrentSsoSettingsGet.g.cs (1)
22-82: Robust implementation of SSO settings retrieval.The method
GetCurrentSsoSettingsApiV1OrgsCurrentSsoSettingsGetAsyncis well-designed and implemented:
- It includes comprehensive steps for preparing, sending, and processing HTTP requests.
- Exception handling is robust, ensuring that HTTP errors are managed effectively.
- The use of
ConfigureAwait(false)is appropriate for avoiding deadlocks in library code.This method effectively supports the new API endpoints for managing SSO settings as described in the PR summary.
src/libs/LangSmith/Generated/LangSmith.Models.OrgFeatureFlags.g.cs (2)
114-114: Approved: UpdatedToValueStringmethod.The update to include the
ArbitraryCodeExecutionEnabledenum value in theToValueStringmethod is correct and follows the established pattern.
142-142: Approved: UpdatedToEnummethod.The update to include the reverse mapping for
"arbitrary_code_execution_enabled"toArbitraryCodeExecutionEnabledin theToEnummethod is correct and follows the established pattern.src/libs/LangSmith/Generated/LangSmith.Models.RunRulesCreateSchema.g.cs (1)
121-127: Well-implemented new property addition.The addition of the
CodeEvaluatorsproperty to theRunRulesCreateSchemaclass is well-implemented. The use of theAnyOftype enhances flexibility, and the JSON serialization attributes are correctly applied to ensure proper data handling.src/libs/LangSmith/Generated/LangSmith.RunClient.CreateRuleApiV1RunsRulesPost.g.cs (1)
Line range hint
126-148: Enhanced method functionality with new parameter addition.The addition of the
codeEvaluatorsparameter to theProcessCreateRuleApiV1RunsRulesPostResponseContentmethod enhances its functionality, allowing it to handle a broader range of evaluators. The updates to the method's documentation and signature are well-executed, ensuring clarity and maintainability.Also applies to: 171-171
src/libs/LangSmith/Generated/LangSmith.RunClient.UpdateRuleApiV1RunsRulesRuleIdPatch.g.cs (1)
Line range hint
133-156: Enhanced method functionality with new parameter addition.The addition of the
codeEvaluatorsparameter to theProcessUpdateRuleApiV1RunsRulesRuleIdPatchResponseContentmethod enhances its functionality, allowing it to handle code evaluators alongside the existing evaluators, alerts, and webhooks. The updates to the method's documentation and signature are well-executed, ensuring clarity and maintainability.Also applies to: 179-179
src/libs/LangSmith/openapi.yaml (6)
1309-1329: Review of GET/api/v1/orgs/current/sso-settingsEndpointThis new endpoint is well-defined with appropriate tags, summary, description, and operation ID. The response schema correctly references the
SSOProviderschema, which should be reviewed for accuracy in its definition. Security requirements are clearly specified, ensuring that appropriate authentication is enforced.
1330-1359: Review of POST/api/v1/orgs/current/sso-settingsEndpointThis endpoint for creating SSO settings is correctly set up with comprehensive details including tags, summary, description, and operation ID. The request body schema
SSOSettingsCreateneeds to be validated to ensure it meets the requirements for creating SSO settings. The response and error handling are appropriately defined, and security requirements are specified.
3444-3472: Review of POST/api/v1/runs/rules/triggerEndpointThis endpoint allows for triggering an array of run rules manually, which is a significant addition for operational flexibility. The request body schema
TriggerRulesRequestis used here and should be checked for correctness. The endpoint is secured with API key and tenant ID, which is appropriate for such operations.
16306-16330: Review ofSSOProviderSchemaThe
SSOProviderschema is well-structured with necessary fields such asid,provider_id,metadata_url, andmetadata_xml. Each field is appropriately typed, and the schema is used in the GET endpoint for SSO settings. Ensure that the fields meet the integration requirements of SSO providers.
16332-16349: Review ofSSOSettingsCreateSchemaThis schema for creating SSO settings includes fields for
metadata_xml,metadata_url, andattribute_mapping. It is crucial that these fields are validated against the SSO provider's requirements. The schema is used in the POST endpoint for creating SSO settings, and it is essential to ensure that it captures all necessary details for a successful SSO configuration.
17865-17876: Review ofTriggerRulesRequestSchemaThe
TriggerRulesRequestschema is designed to handle the request for triggering rules manually. It includes an array ofrule_ids, which are expected to be in UUID format. This schema is crucial for the operation of the new POST endpoint for triggering rules and should be validated to ensure that it correctly handles the rule IDs.
| namespace LangSmith | ||
| { | ||
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| public sealed partial class SSOProvider | ||
| { | ||
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| [global::System.Text.Json.Serialization.JsonPropertyName("id")] | ||
| [global::System.Text.Json.Serialization.JsonRequired] | ||
| public required string Id { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| [global::System.Text.Json.Serialization.JsonPropertyName("provider_id")] | ||
| [global::System.Text.Json.Serialization.JsonRequired] | ||
| public required string ProviderId { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| [global::System.Text.Json.Serialization.JsonPropertyName("metadata_url")] | ||
| [global::System.Text.Json.Serialization.JsonConverter(typeof(global::OpenApiGenerator.JsonConverters.AnyOfJsonConverterFactory2))] | ||
| public global::System.AnyOf<string, object>? MetadataUrl { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| [global::System.Text.Json.Serialization.JsonPropertyName("metadata_xml")] | ||
| [global::System.Text.Json.Serialization.JsonConverter(typeof(global::OpenApiGenerator.JsonConverters.AnyOfJsonConverterFactory2))] | ||
| public global::System.AnyOf<string, object>? MetadataXml { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Additional properties that are not explicitly defined in the schema | ||
| /// </summary> | ||
| [global::System.Text.Json.Serialization.JsonExtensionData] | ||
| public global::System.Collections.Generic.IDictionary<string, object> AdditionalProperties { get; set; } = new global::System.Collections.Generic.Dictionary<string, object>(); | ||
| } |
There was a problem hiding this comment.
Well-structured class definition for SSOProvider.
The class SSOProvider is well-defined with appropriate use of attributes and properties:
- The use of
sealedandpartialis appropriate for generated classes. - Properties
IdandProviderIdare correctly marked as required. - The use of custom JSON converters for
MetadataUrlandMetadataXmlis a good practice, providing flexibility in accepted data types. - The
AdditionalPropertiesdictionary enhances the class's ability to handle data beyond the predefined schema.
Suggestion: Add documentation.
The summary tags for properties are empty. Adding descriptive comments would improve the readability and maintainability of the code.
| /// <summary> | ||
| /// | ||
| /// </summary> | ||
| ArbitraryCodeExecutionEnabled, |
There was a problem hiding this comment.
Add documentation for the new enum value.
The enum value ArbitraryCodeExecutionEnabled is added correctly, but it lacks a summary documentation. This is crucial for understanding its purpose and implications, especially given its sensitive nature.
Consider adding a detailed description in the summary section:
Committable suggestion was skipped due to low confidence.
| public async global::System.Threading.Tasks.Task<global::LangSmith.TriggerRulesApiV1RunsRulesTriggerPostResponse> TriggerRulesApiV1RunsRulesTriggerPostAsync( | ||
| global::LangSmith.TriggerRulesRequest request, | ||
| global::System.Threading.CancellationToken cancellationToken = default) | ||
| { | ||
| request = request ?? throw new global::System.ArgumentNullException(nameof(request)); | ||
|
|
||
| PrepareArguments( | ||
| client: _httpClient); | ||
| PrepareTriggerRulesApiV1RunsRulesTriggerPostArguments( | ||
| httpClient: _httpClient, | ||
| request: request); | ||
|
|
||
| using var httpRequest = new global::System.Net.Http.HttpRequestMessage( | ||
| method: global::System.Net.Http.HttpMethod.Post, | ||
| requestUri: new global::System.Uri(_httpClient.BaseAddress?.AbsoluteUri.TrimEnd('/') + "/api/v1/runs/rules/trigger", global::System.UriKind.RelativeOrAbsolute)); | ||
| var __httpRequestContentBody = global::System.Text.Json.JsonSerializer.Serialize(request, global::LangSmith.SourceGenerationContext.Default.TriggerRulesRequest); | ||
| var __httpRequestContent = new global::System.Net.Http.StringContent( | ||
| content: __httpRequestContentBody, | ||
| encoding: global::System.Text.Encoding.UTF8, | ||
| mediaType: "application/json"); | ||
| httpRequest.Content = __httpRequestContent; | ||
|
|
||
| PrepareRequest( | ||
| client: _httpClient, | ||
| request: httpRequest); | ||
| PrepareTriggerRulesApiV1RunsRulesTriggerPostRequest( | ||
| httpClient: _httpClient, | ||
| httpRequestMessage: httpRequest, | ||
| request: request); | ||
|
|
||
| using var response = await _httpClient.SendAsync( | ||
| request: httpRequest, | ||
| completionOption: global::System.Net.Http.HttpCompletionOption.ResponseContentRead, | ||
| cancellationToken: cancellationToken).ConfigureAwait(false); | ||
|
|
||
| ProcessResponse( | ||
| client: _httpClient, | ||
| response: response); | ||
| ProcessTriggerRulesApiV1RunsRulesTriggerPostResponse( | ||
| httpClient: _httpClient, | ||
| httpResponseMessage: response); | ||
|
|
||
| var __content = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| ProcessResponseContent( | ||
| client: _httpClient, | ||
| response: response, | ||
| content: ref __content); | ||
| ProcessTriggerRulesApiV1RunsRulesTriggerPostResponseContent( | ||
| httpClient: _httpClient, | ||
| httpResponseMessage: response, | ||
| content: ref __content); | ||
|
|
||
| try | ||
| { | ||
| response.EnsureSuccessStatusCode(); | ||
| } | ||
| catch (global::System.Net.Http.HttpRequestException ex) | ||
| { | ||
| throw new global::System.InvalidOperationException(__content, ex); | ||
| } | ||
|
|
||
| return | ||
| global::System.Text.Json.JsonSerializer.Deserialize(__content, global::LangSmith.SourceGenerationContext.Default.TriggerRulesApiV1RunsRulesTriggerPostResponse) ?? | ||
| throw new global::System.InvalidOperationException($"Response deserialization failed for \"{__content}\" "); | ||
| } |
There was a problem hiding this comment.
Approved: Well-structured API method with robust error handling.
The TriggerRulesApiV1RunsRulesTriggerPostAsync method is well-structured and includes robust error handling and response processing. However, consider adding more detailed error logging to aid in debugging and maintaining the service.
Consider enhancing error logging:
Committable suggestion was skipped due to low confidence.
| public async global::System.Threading.Tasks.Task<global::LangSmith.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse> CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostAsync( | ||
| global::LangSmith.SSOSettingsCreate request, | ||
| global::System.Threading.CancellationToken cancellationToken = default) | ||
| { | ||
| request = request ?? throw new global::System.ArgumentNullException(nameof(request)); | ||
|
|
||
| PrepareArguments( | ||
| client: _httpClient); | ||
| PrepareCreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostArguments( | ||
| httpClient: _httpClient, | ||
| request: request); | ||
|
|
||
| using var httpRequest = new global::System.Net.Http.HttpRequestMessage( | ||
| method: global::System.Net.Http.HttpMethod.Post, | ||
| requestUri: new global::System.Uri(_httpClient.BaseAddress?.AbsoluteUri.TrimEnd('/') + "/api/v1/orgs/current/sso-settings", global::System.UriKind.RelativeOrAbsolute)); | ||
| var __httpRequestContentBody = global::System.Text.Json.JsonSerializer.Serialize(request, global::LangSmith.SourceGenerationContext.Default.SSOSettingsCreate); | ||
| var __httpRequestContent = new global::System.Net.Http.StringContent( | ||
| content: __httpRequestContentBody, | ||
| encoding: global::System.Text.Encoding.UTF8, | ||
| mediaType: "application/json"); | ||
| httpRequest.Content = __httpRequestContent; | ||
|
|
||
| PrepareRequest( | ||
| client: _httpClient, | ||
| request: httpRequest); | ||
| PrepareCreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostRequest( | ||
| httpClient: _httpClient, | ||
| httpRequestMessage: httpRequest, | ||
| request: request); | ||
|
|
||
| using var response = await _httpClient.SendAsync( | ||
| request: httpRequest, | ||
| completionOption: global::System.Net.Http.HttpCompletionOption.ResponseContentRead, | ||
| cancellationToken: cancellationToken).ConfigureAwait(false); | ||
|
|
||
| ProcessResponse( | ||
| client: _httpClient, | ||
| response: response); | ||
| ProcessCreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse( | ||
| httpClient: _httpClient, | ||
| httpResponseMessage: response); | ||
|
|
||
| var __content = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| ProcessResponseContent( | ||
| client: _httpClient, | ||
| response: response, | ||
| content: ref __content); | ||
| ProcessCreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponseContent( | ||
| httpClient: _httpClient, | ||
| httpResponseMessage: response, | ||
| content: ref __content); | ||
|
|
||
| try | ||
| { | ||
| response.EnsureSuccessStatusCode(); | ||
| } | ||
| catch (global::System.Net.Http.HttpRequestException ex) | ||
| { | ||
| throw new global::System.InvalidOperationException(__content, ex); | ||
| } | ||
|
|
||
| return | ||
| global::System.Text.Json.JsonSerializer.Deserialize(__content, global::LangSmith.SourceGenerationContext.Default.CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostResponse) ?? | ||
| throw new global::System.InvalidOperationException($"Response deserialization failed for \"{__content}\" "); | ||
| } |
There was a problem hiding this comment.
Approved: Well-structured API method with robust error handling.
The CreateSsoSettingsApiV1OrgsCurrentSsoSettingsPostAsync method is well-structured and includes robust error handling and response processing. However, consider adding more detailed error logging to aid in debugging and maintaining the service.
Consider enhancing error logging:
Committable suggestion was skipped due to low confidence.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation