-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat:Enhance LangSmith library with custom chart handling and API updates #28
Conversation
WalkthroughThe changes introduced enhancements to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChartsClient
participant API
User->>ChartsClient: Request chart preview
ChartsClient->>API: POST /api/v1/charts/preview
API->>ChartsClient: Response with chart preview data
ChartsClient->>User: Return chart preview
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
src/libs/LangSmith/Generated/LangSmith.Models.CustomChartCreatePreview.g.cs (1)
9-16
: Ensure Validation and Documentation forSeries
PropertyThe
Series
property in theCustomChartCreatePreview
class is marked as required, but there is no explicit validation logic or detailed documentation found in the codebase. Please ensure that this requirement is validated in the API layer and properly documented.
- Verify that the
Series
property is validated to ensure it is not null or empty.- Ensure that the requirement for the
Series
property is documented in the API documentation.Analysis chain
Introduction of
CustomChartCreatePreview
class.The new class
CustomChartCreatePreview
includes a requiredSeries
property, which must be provided in JSON payloads. Ensure that this requirement is documented and validated in the API layer.Run the following script to verify the usage of the
Series
property:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and validation of the `Series` property in the codebase. # Test: Search for the `Series` property usage. Expect: Proper validation and documentation. rg --type cs 'Series'Length of output: 4447
Script:
#!/bin/bash # Search for validation logic related to the `Series` property. rg --type cs 'Series' -A 5 | grep -iE 'validate|check|require' # Search for documentation comments related to the `Series` property. rg --type cs '///' -A 3 | grep -i 'Series'Length of output: 24030
src/libs/LangSmith/openapi.yaml (1)
8003-8012
: The old/ok
endpoint still exists and should be removed.The old
/ok
endpoint is present indocs/openapi.yaml
, while the new/api/v1/ok
endpoint is defined insrc/libs/LangSmith/openapi.yaml
. Ensure the old endpoint is removed to avoid conflicts or redundancy.
- File:
docs/openapi.yaml
- Endpoint:
/ok
Analysis chain
New endpoint
/api/v1/ok
looks good.The endpoint provides a simple successful response and replaces the old
/ok
endpoint.Ensure that the old
/ok
endpoint has been removed from the specification.Run the following script to verify the removal of the old
/ok
endpoint:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of the old `/ok` endpoint. # Test: Search for the old endpoint definition. Expect: No occurrences of the old `/ok` endpoint. rg --type yaml $'/ok:\n' -A 5Length of output: 221
Script:
#!/bin/bash # Description: Verify the removal of the old `/ok` endpoint. # Test: Search for the old endpoint definition. Expect: No occurrences of the old `/ok` endpoint. rg --type yaml --multiline $'/ok:\n' -A 5Length of output: 545
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- src/libs/LangSmith/Generated/LangSmith.ChartsClient.ReadChartPreviewApiV1ChartsPreviewPost.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.LangSmithApi.OkApiV1OkGet.g.cs (5 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.CustomChartCreatePreview.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.CustomChartPreviewRequest.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.CustomChartsRequestBase.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.OkApiV1OkGetResponse.g.cs (1 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.SingleCustomChartResponse.g.cs (2 hunks)
- src/libs/LangSmith/Generated/LangSmith.Models.SingleCustomChartResponseBase.g.cs (1 hunks)
- src/libs/LangSmith/openapi.yaml (8 hunks)
Additional context used
GitHub Check: Test / Build, test and publish
src/libs/LangSmith/Generated/LangSmith.ChartsClient.ReadChartPreviewApiV1ChartsPreviewPost.g.cs
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
Additional comments not posted (24)
src/libs/LangSmith/Generated/LangSmith.Models.OkApiV1OkGetResponse.g.cs (1)
9-9
: Class name change reflects versioning update.The class name has been updated to
OkApiV1OkGetResponse
, indicating a versioned API response model. Ensure that all references to the old class name are updated throughout the codebase.Run the following script to verify the usage of the old class name:
Verification successful
No references to the old class name
OkOkGetResponse
found.The search did not return any occurrences of the old class name, indicating that all references have been updated to
OkApiV1OkGetResponse
. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the old class name `OkOkGetResponse`. # Test: Search for the old class name usage. Expect: No occurrences of the old class name. rg --type cs 'OkOkGetResponse'Length of output: 30
Script:
#!/bin/bash # Re-run the search for the old class name `OkOkGetResponse` to verify its presence in the codebase. rg --type cs 'OkOkGetResponse'Length of output: 30
src/libs/LangSmith/Generated/LangSmith.Models.SingleCustomChartResponseBase.g.cs (1)
9-16
: Introduction ofSingleCustomChartResponseBase
class.The new class
SingleCustomChartResponseBase
includes a requiredData
property, which must be provided in JSON payloads. Ensure that this requirement is documented and validated in the API layer.Run the following script to verify the usage of the
Data
property:src/libs/LangSmith/Generated/LangSmith.Models.CustomChartPreviewRequest.g.cs (3)
14-16
: PropertyBucketInfo
is correctly defined.The property is marked as required and uses the custom type
CustomChartsRequestBase
, which is appropriate for its intended use.
21-23
: PropertyChart
is correctly defined.The property is marked as required and uses the custom type
CustomChartCreatePreview
, which is appropriate for its intended use.
28-29
: PropertyAdditionalProperties
is correctly defined.The property uses the
JsonExtensionData
attribute to handle additional data, which is appropriate for its intended use.src/libs/LangSmith/Generated/LangSmith.Models.CustomChartsRequestBase.g.cs (5)
16-17
: PropertyTimezone
is correctly defined.The property is optional and defaults to "UTC", which is appropriate for its intended use.
22-24
: PropertyStartTime
is correctly defined.The property is marked as required and uses the
DateTime
type, which is appropriate for its intended use.
29-31
: PropertyEndTime
is correctly defined.The property uses a custom converter and supports multiple types, which is appropriate for its intended use.
36-38
: PropertyStride
is correctly defined.The property uses a custom converter and supports multiple types, which is appropriate for its intended use.
43-44
: PropertyAdditionalProperties
is correctly defined.The property uses the
JsonExtensionData
attribute to handle additional data, which is appropriate for its intended use.src/libs/LangSmith/Generated/LangSmith.Models.SingleCustomChartResponse.g.cs (1)
16-18
: PropertyData
is correctly defined.The property is marked as required and uses a list of
CustomChartsDataPoint
, which is appropriate for its intended use.src/libs/LangSmith/Generated/LangSmith.LangSmithApi.OkApiV1OkGet.g.cs (6)
10-12
: Ensure consistency in method renaming.The method
PrepareOkApiV1OkGetRequest
has been renamed to align with the versioned API structure. Verify that all references to this method in the codebase have been updated accordingly.Run the following script to verify the method usage:
Verification successful
Method renaming is consistent.
The method
PrepareOkApiV1OkGetRequest
is only referenced within the file where it is defined, and no other occurrences exist in the codebase. Thus, the renaming is consistent.
- File:
src/libs/LangSmith/Generated/LangSmith.LangSmithApi.OkApiV1OkGet.g.cs
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `PrepareOkApiV1OkGetRequest` are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method name. rg --type cs $'PrepareOkApiV1OkGetRequest'Length of output: 270
8-9
: Ensure consistency in method renaming.The method
PrepareOkApiV1OkGetArguments
has been renamed to align with the versioned API structure. Verify that all references to this method in the codebase have been updated accordingly.Run the following script to verify the method usage:
17-19
: Ensure consistency in method renaming.The method
ProcessOkApiV1OkGetResponseContent
has been renamed to align with the versioned API structure. Verify that all references to this method in the codebase have been updated accordingly.Run the following script to verify the method usage:
Verification successful
Method renaming is consistent.
The method
ProcessOkApiV1OkGetResponseContent
is only defined and used within the filesrc/libs/LangSmith/Generated/LangSmith.LangSmithApi.OkApiV1OkGet.g.cs
. There are no other references to this method elsewhere in the codebase, indicating that the renaming is consistent.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `ProcessOkApiV1OkGetResponseContent` are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method name. rg --type cs $'ProcessOkApiV1OkGetResponseContent'Length of output: 294
Script:
#!/bin/bash # Comprehensive search for the method `ProcessOkApiV1OkGetResponseContent` across the entire codebase. rg 'ProcessOkApiV1OkGetResponseContent' --type cs -A 3Length of output: 1020
13-15
: Ensure consistency in method renaming.The method
ProcessOkApiV1OkGetResponse
has been renamed to align with the versioned API structure. Verify that all references to this method in the codebase have been updated accordingly.Run the following script to verify the method usage:
Verification successful
Method Renaming Verified: No Inconsistent References Found
The method
ProcessOkApiV1OkGetResponse
is only referenced within the fileLangSmith.LangSmithApi.OkApiV1OkGet.g.cs
, and all occurrences are consistent with the renaming. No other references exist in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `ProcessOkApiV1OkGetResponse` are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method name. rg --type cs $'ProcessOkApiV1OkGetResponse'Length of output: 518
Script:
#!/bin/bash # Search for all occurrences of the method `ProcessOkApiV1OkGetResponse` across the entire codebase. rg --type cs 'ProcessOkApiV1OkGetResponse'Length of output: 518
27-29
: Ensure consistency in method renaming.The method
OkApiV1OkGetAsync
has been renamed to align with the versioned API structure. Verify that all references to this method in the codebase have been updated accordingly.Run the following script to verify the method usage:
Verification successful
Method Renaming Consistency Verified
The method
OkApiV1OkGetAsync
is not referenced elsewhere in the codebase, indicating that the renaming is consistent. No outdated references were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `OkApiV1OkGetAsync` are updated. # Test: Search for the method usage. Expect: Only occurrences of the new method name. rg --type cs $'OkApiV1OkGetAsync'Length of output: 218
Script:
#!/bin/bash # Description: Verify all references to `OkApiV1OkGetAsync` are updated across all file types. # Test: Search for the method usage in all file types. Expect: Only occurrences of the new method name. rg 'OkApiV1OkGetAsync'Length of output: 208
37-37
: Verify the updated request URI.The request URI has been updated to
"/api/v1/ok"
. Ensure that this change is consistent with the intended API versioning strategy.Run the following script to verify the request URI usage:
src/libs/LangSmith/openapi.yaml (7)
8000-8002
: No changes detected in the security schemes section.There are no changes to review in this hunk.
16310-16315
: New schemaSingleCustomChartResponseBase
looks good.The schema is well-defined with a required
data
property, aligning with the new functionality.No further actions required.
7770-7799
: New endpoint/api/v1/charts/preview
looks good.The endpoint is well-defined with appropriate request and response structures.
Ensure that the referenced schemas
CustomChartPreviewRequest
andSingleCustomChartResponseBase
are correctly defined in the components section.Run the following script to verify the existence of referenced schemas:
Verification successful
Schemas
CustomChartPreviewRequest
andSingleCustomChartResponseBase
are correctly defined.The endpoint
/api/v1/charts/preview
references these schemas appropriately, and they are well-structured in theopenapi.yaml
file.
CustomChartPreviewRequest
is defined with required fieldsbucket_info
andchart
.SingleCustomChartResponseBase
includes a requireddata
field.No issues found with the schema definitions or their usage in the endpoint.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of referenced schemas in the components section. # Test: Search for the schema definitions. Expect: Definitions for both schemas. rg --type yaml $'CustomChartPreviewRequest:\n' -A 10 rg --type yaml $'SingleCustomChartResponseBase:\n' -A 10Length of output: 493
Script:
#!/bin/bash # Description: Verify the existence of referenced schemas in the components section with multiline mode enabled. # Test: Search for the schema definitions using multiline mode. rg --type yaml --multiline 'CustomChartPreviewRequest:\n' -A 10 rg --type yaml --multiline 'SingleCustomChartResponseBase:\n' -A 10Length of output: 1447
10317-10327
: New schemaCustomChartPreviewRequest
looks good.The schema is well-defined with required properties
bucket_info
andchart
.Ensure that the referenced schemas
CustomChartsRequestBase
andCustomChartCreatePreview
are correctly defined in the components section.Run the following script to verify the existence of referenced schemas:
Verification successful
New schema
CustomChartPreviewRequest
looks good.The schema is well-defined with required properties
bucket_info
andchart
. The referenced schemasCustomChartsRequestBase
andCustomChartCreatePreview
are correctly defined in the components section.
CustomChartsRequestBase
is defined with properties liketimezone
andstart_time
.CustomChartCreatePreview
is defined with aseries
property.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of referenced schemas in the components section. # Test: Search for the schema definitions. Expect: Definitions for both schemas. rg --type yaml $'CustomChartsRequestBase:\n' -A 10 rg --type yaml $'CustomChartCreatePreview:\n' -A 10Length of output: 486
Script:
#!/bin/bash # Description: Verify the existence of referenced schemas in the components section. # Test: Search for the schema definitions. Expect: Definitions for both schemas. rg --type yaml --multiline 'CustomChartsRequestBase:' -A 10 rg --type yaml --multiline 'CustomChartCreatePreview:' -A 10Length of output: 1350
10284-10294
: New schemaCustomChartCreatePreview
looks good.The schema is well-defined with a required
series
property.Ensure that the referenced schema
CustomChartSeries
is correctly defined in the components section.Run the following script to verify the existence of the referenced schema:
Verification successful
Schema reference
CustomChartSeries
is correctly defined.The
CustomChartSeries
schema is present and well-structured, confirming the validity of its reference in theCustomChartCreatePreview
schema.
CustomChartSeries
is defined with required properties:name
,filters
,metric
, andid
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `CustomChartSeries` schema in the components section. # Test: Search for the schema definition. Expect: Definition for `CustomChartSeries`. rg --type yaml $'CustomChartSeries:\n' -A 10Length of output: 236
Script:
#!/bin/bash # Description: Verify the existence of the `CustomChartSeries` schema in the components section. # Test: Search for the schema definition with multiline mode enabled. Expect: Definition for `CustomChartSeries`. rg --type yaml --multiline 'CustomChartSeries:\n' -A 10Length of output: 615
16270-16282
: Modification toSingleCustomChartResponse
looks good.The schema now requires the
data
property, which aligns with the new functionality.Ensure that the referenced schema
CustomChartsDataPoint
is correctly defined in the components section.Run the following script to verify the existence of the referenced schema:
Verification successful
The
CustomChartsDataPoint
schema is correctly defined.The schema is present in the
openapi.yaml
file with necessary properties, confirming the validity of the reference in theSingleCustomChartResponse
schema.
- The
CustomChartsDataPoint
schema includes properties likeseries_id
,timestamp
, andvalue
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `CustomChartsDataPoint` schema in the components section. # Test: Search for the schema definition. Expect: Definition for `CustomChartsDataPoint`. rg --type yaml $'CustomChartsDataPoint:\n' -A 10Length of output: 240
Script:
#!/bin/bash # Description: Verify the existence of the `CustomChartsDataPoint` schema in the components section. # Test: Search for the schema definition using multiline mode. rg --type yaml --multiline 'CustomChartsDataPoint:\n' -A 10Length of output: 653
10584-10610
: New schemaCustomChartsRequestBase
looks good.The schema is well-defined with required
start_time
property and other properties liketimezone
,end_time
, andstride
.Ensure that the referenced schema
TimedeltaInput
is correctly defined in the components section.Run the following script to verify the existence of the referenced schema:
Verification successful
The
TimedeltaInput
schema is correctly defined.The
TimedeltaInput
schema exists in theopenapi.yaml
file with the expected properties, confirming the validity of the reference in theCustomChartsRequestBase
schema.
- The schema includes properties like
days
andminutes
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the `TimedeltaInput` schema in the components section. # Test: Search for the schema definition. Expect: Definition for `TimedeltaInput`. rg --type yaml $'TimedeltaInput:\n' -A 10Length of output: 233
Script:
#!/bin/bash # Description: Verify the existence of the `TimedeltaInput` schema in the components section. # Test: Search for the schema definition. Expect: Definition for `TimedeltaInput`. rg --type yaml 'TimedeltaInput:' -A 10Length of output: 622
global::System.Text.Json.JsonSerializer.Deserialize(__content, global::LangSmith.SourceGenerationContext.Default.NullableSingleCustomChartResponseBase) ?? | ||
throw new global::System.InvalidOperationException($"Response deserialization failed for \"{__content}\" "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the missing definition for NullableSingleCustomChartResponseBase
.
The static analysis tool indicates that SourceGenerationContext
does not contain a definition for NullableSingleCustomChartResponseBase
. Ensure that this type is defined or correctly referenced.
Please verify the existence of NullableSingleCustomChartResponseBase
in SourceGenerationContext
or update the reference to the correct type.
Tools
GitHub Check: Test / Build, test and publish
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
[failure] 94-94:
'SourceGenerationContext' does not contain a definition for 'NullableSingleCustomChartResponseBase' and no accessible extension method 'NullableSingleCustomChartResponseBase' accepting a first argument of type 'SourceGenerationContext' could be found (are you missing a using directive or an assembly reference?)
Summary by CodeRabbit
New Features
Enhancements
Documentation