-
-
Notifications
You must be signed in to change notification settings - Fork 0
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:Add documents
parameter and Document
class to Cohere API
#50
Conversation
WalkthroughThe changes introduce a new parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Model
participant Document
User->>API: Send request with documents
API->>Model: Process request with documents
Model->>Document: Reference relevant documents
Document-->>Model: Return document data
Model-->>API: Generate response
API-->>User: Return response
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 (
|
documents
parameter and Document
class to Cohere API
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: 24
Outside diff range and nitpick comments (12)
src/libs/Cohere/Generated/Cohere.Models.DocumentContentType.g.cs (4)
6-8
: Add meaningful XML documentation to 'DocumentContentType' enumThe XML documentation for the
DocumentContentType
enum is currently empty. Adding a meaningful summary will enhance code readability and maintainability.
11-13
: Provide summary for 'Document' enum memberThe XML documentation for the
Document
enum member is missing. Consider adding a description to explain its purpose.
18-19
: Correct grammatical error in summary commentThe comment "Enum extensions to do fast conversions without the reflection." should be corrected for grammar by removing "the" to read "without reflection."
Apply this diff to fix the grammatical error:
- /// Enum extensions to do fast conversions without the reflection. + /// Enum extensions to do fast conversions without reflection.
34-35
: Fix grammatical errors in documentation commentThe summary comment "Converts an string to a enum." contains grammatical errors. It should read "Converts a string to an enum."
Apply this diff to correct the grammar:
- /// Converts an string to a enum. + /// Converts a string to an enum.src/libs/Cohere/Generated/Cohere.Models.DocumentContent.g.cs (1)
11-13
: Add XML documentation for theType
propertyThe
Type
property is missing summary documentation. Providing a summary enhances code readability and helps maintain consistency in documentation.src/libs/Cohere/Generated/JsonConverters.DocumentContentTypeNullable.g.cs (2)
18-24
: Simplify null check in 'JsonTokenType.String' caseSince
reader.GetString()
will not return null whenTokenType
isJsonTokenType.String
, the null check and thebreak
statement are unnecessary. You can simplify the code by removing them.Apply this diff to simplify the code:
var stringValue = reader.GetString(); -if (stringValue != null) -{ return global::Cohere.DocumentContentTypeExtensions.ToEnum(stringValue); -} - -break; +return global::Cohere.DocumentContentTypeExtensions.ToEnum(stringValue);
44-44
: Remove redundant null check and assignment for 'writer'Since
writer
is a non-nullable parameter, the null check and assignment are unnecessary.You can remove this line:
-writer = writer ?? throw new global::System.ArgumentNullException(nameof(writer));
src/libs/Cohere/Generated/Cohere.Models.ToolContent.g.cs (1)
22-24
: Provide meaningful XML documentation commentsSeveral public members have empty XML documentation comments. Providing descriptive summaries enhances code readability and helps generate comprehensive documentation for users of the API.
Also applies to: 30-32, 35-37, 40-42, 57-59, 65-67, 70-72, 75-77, 83-85, 95-97, 103-105, 111-113, 131-133, 142-144, 150-152, 158-160
src/libs/Cohere/Generated/Cohere.Models.Chatv2Request.g.cs (1)
35-39
: Enhance documentation with usage examplesConsider adding examples to the XML documentation comments for the
Documents
property. Providing sample code on how to use this property with bothstring
andDocument
instances will improve developer understanding and ease of use.src/libs/Cohere/Generated/Cohere.CohereApi.Chatv2.g.cs (1)
113-115
: Clarify the parameter documentation for better readabilityThe documentation for the
documents
parameter can be improved by formattingDocument
as code and specifying that it's a class. This enhances readability and consistency with other parameter descriptions.Apply this diff to improve the documentation:
/// <param name="documents"> -/// A list of relevant documents that the model can cite to generate a more accurate reply. Each document is either a string or document object with content and metadata. +/// A list of relevant documents that the model can cite to generate a more accurate reply. Each document is either a string or a `Document` object containing content and metadata. /// </param>src/libs/Cohere/Generated/Cohere.Models.Document.g.cs (1)
24-24
: Consider defaulting theId
property to prevent null referencesThe
Id
property is nullable (string?
). To avoid potential null reference issues, consider initializing it with a default value or ensuring that any code consuming this property handlesnull
values appropriately.src/libs/Cohere/Generated/Cohere.Models.ToolMessage2.g.cs (1)
28-28
: Typo in XML Documentation CommentThere's a missing word in the summary comment:
- "The content should formatted as a JSON object string..."
- Should be: "The content should be formatted as a JSON object string..."
Apply this diff to fix the typo:
- /// A single or list of outputs from a tool. The content should formatted as a JSON object string, or a list of tool content blocks + /// A single or list of outputs from a tool. The content should be formatted as a JSON object string, or a list of tool content blocks
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/libs/Cohere/Generated/Cohere.CohereApi.Chatv2.g.cs (3 hunks)
- src/libs/Cohere/Generated/Cohere.ICohereApi.Chatv2.g.cs (2 hunks)
- src/libs/Cohere/Generated/Cohere.Models.Chatv2Request.g.cs (2 hunks)
- src/libs/Cohere/Generated/Cohere.Models.Document.g.cs (1 hunks)
- src/libs/Cohere/Generated/Cohere.Models.DocumentContent.g.cs (1 hunks)
- src/libs/Cohere/Generated/Cohere.Models.DocumentContentType.g.cs (1 hunks)
- src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs (1 hunks)
- src/libs/Cohere/Generated/Cohere.Models.ToolContent.g.cs (1 hunks)
- src/libs/Cohere/Generated/Cohere.Models.ToolMessage2.g.cs (2 hunks)
- src/libs/Cohere/Generated/Cohere.Models.UserMessage.g.cs (0 hunks)
- src/libs/Cohere/Generated/JsonConverters.DocumentContentType.g.cs (1 hunks)
- src/libs/Cohere/Generated/JsonConverters.DocumentContentTypeNullable.g.cs (1 hunks)
- src/libs/Cohere/Generated/JsonConverters.ToolContent.g.cs (1 hunks)
- src/libs/Cohere/Generated/JsonSerializerContext.g.cs (2 hunks)
- src/libs/Cohere/openapi.yaml (14 hunks)
Files not reviewed due to no reviewable changes (1)
- src/libs/Cohere/Generated/Cohere.Models.UserMessage.g.cs
Additional comments not posted (11)
src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs (1)
9-17
: LGTM!The class is well-defined and follows best practices.
src/libs/Cohere/Generated/Cohere.ICohereApi.Chatv2.g.cs (1)
33-35
: Parameter documentation is clear and informativeThe added XML documentation for the
documents
parameter is well-written and provides a clear description of its purpose and usage.src/libs/Cohere/Generated/Cohere.Models.Chatv2Request.g.cs (1)
2-2
: Confirm the necessity of suppressing obsolete warningsThe
#pragma warning disable CS0618
directive suppresses warnings about obsolete types or members. Please verify that this suppression is intentional and that any obsolete members in use are necessary. It's generally better to update obsolete code to use the recommended alternatives to maintain code health and future compatibility.src/libs/Cohere/Generated/Cohere.CohereApi.Chatv2.g.cs (1)
194-194
: Ensure thedocuments
property is assigned correctly after parameter repositioningAfter moving the
documents
parameter to the end, make sure it is still correctly assigned to theDocuments
property in the request object.var request = new global::Cohere.Chatv2Request { Model = model, Messages = messages, Tools = tools, CitationMode = citationMode, ResponseFormat = responseFormat, SafetyMode = safetyMode, MaxTokens = maxTokens, StopSequences = stopSequences, Temperature = temperature, Seed = seed, FrequencyPenalty = frequencyPenalty, PresencePenalty = presencePenalty, K = k, P = p, + Documents = documents, };
src/libs/Cohere/openapi.yaml (2)
761-767
: Ensure compatibility ofdocuments
field with tooling and SDKsThe
documents
field usesoneOf
within an array to allow each item to be either a string or aDocument
object. While this is valid in OpenAPI, some code generators and SDKs may not handle polymorphic arrays well. This could lead to integration issues or unexpected behavior when developers use the generated client libraries.
7600-7605
: Verifytool_content
field definition for consistencyThe
tool_content
field inToolMessage-2
is defined as aoneOf
:
- A string
- An array of
ToolContent
objectsEnsure that this definition aligns with how
tool_content
is used elsewhere in the API specification and that it is consistently represented in the SDKs. Inconsistent definitions may lead to confusion or errors in client implementations.src/libs/Cohere/Generated/Cohere.Models.Document.g.cs (1)
17-17
: Verify the existence ofDocumentData
in the codebaseEnsure that the type
global::Cohere.DocumentData
is defined in the codebase. If it's missing, the code will fail to compile, and you may need to defineDocumentData
or adjust the reference.Run the following script to check if
DocumentData
is defined:Verification successful
To ensure we have all the necessary information, let's verify the full namespace of the DocumentData class and check for any relevant using directives.
To ensure we have all the necessary information, let's check for the namespace declarations in both files and look for any global aliases that might affect how DocumentData is referenced.
global::Cohere.DocumentData
exists and is correctly referencedThe verification process has confirmed that the
DocumentData
type exists in the codebase and is correctly referenced asglobal::Cohere.DocumentData
. Here are the key findings:
- The
DocumentData
class is defined insrc/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs
.- Both
DocumentData
andDocument
classes are in theCohere
namespace.- In the
Document
class, there's a property that correctly referencesDocumentData
:public required global::Cohere.DocumentData Data { get; set; }The use of the
global::
prefix ensures that the correct type is referenced, avoiding any potential naming conflicts. The code will compile successfully with this reference.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition of `DocumentData` in the codebase. # Test: Search for the declaration of `DocumentData`. # Expect: A definition of `class DocumentData` or `struct DocumentData`. rg --type cs $'\\b(public|internal)?\\s+(class|struct)\\s+DocumentData\\b'Length of output: 174
Script:
#!/bin/bash # Description: Verify the namespace of DocumentData and check for relevant using directives # Check the namespace of DocumentData echo "Namespace of DocumentData:" rg --type cs -A 1 "class DocumentData" src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs echo "\nUsing directives in the file containing DocumentData:" rg --type cs "^using" src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs echo "\nUsing directives in the file under review:" rg --type cs "^using" src/libs/Cohere/Generated/Cohere.Models.Document.g.csLength of output: 592
Script:
#!/bin/bash # Description: Check namespace declarations and global aliases echo "Namespace declaration in DocumentData.g.cs:" rg --type cs "namespace" src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs echo "\nNamespace declaration in Document.g.cs:" rg --type cs "namespace" src/libs/Cohere/Generated/Cohere.Models.Document.g.cs echo "\nChecking for global aliases in DocumentData.g.cs:" rg --type cs "global::" src/libs/Cohere/Generated/Cohere.Models.DocumentData.g.cs echo "\nChecking for global aliases in Document.g.cs:" rg --type cs "global::" src/libs/Cohere/Generated/Cohere.Models.Document.g.cs echo "\nChecking for any references to DocumentData in Document.g.cs:" rg --type cs "DocumentData" src/libs/Cohere/Generated/Cohere.Models.Document.g.csLength of output: 1834
src/libs/Cohere/Generated/Cohere.Models.ToolMessage2.g.cs (2)
2-2
: Reevaluate Suppression of Obsolete WarningThe pragma directive
#pragma warning disable CS0618
suppresses warnings for obsolete types or members. Ensure that suppressing this warning is necessary and won't hide important deprecation notices that should be addressed.
31-33
: Ensure Correct Serialization and Deserialization ofToolContent
The
ToolContent
property type has been updated toOneOf<string, IList<ToolContent>>
, and a customJsonConverter
has been added. Verify that theOneOfJsonConverterFactory2
handles both serialization and deserialization correctly for both cases (string and list ofToolContent
). Consider adding unit tests to confirm all scenarios are properly supported.src/libs/Cohere/Generated/JsonSerializerContext.g.cs (2)
36-37
: New JSON Converters Added for Document Content TypesThe
DocumentContentTypeJsonConverter
andDocumentContentTypeNullableJsonConverter
have been correctly added to the converters list to handle serialization and deserialization of document content types.
135-135
: AddedToolContentJsonConverter
to Converters ListThe
ToolContentJsonConverter
has been appropriately included in the converters list to extend serialization support for tool content.
/// A relevant documents that the model can cite to generate a more accurate reply. Each document is a string-string dictionary. | ||
/// </summary> |
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.
Correct the grammatical error in the summary comment.
The summary comment has a grammatical error that should be corrected for clarity.
Apply this diff to fix the typo:
- /// A relevant documents that the model can cite to generate a more accurate reply. Each document is a string-string dictionary.
+ /// Relevant documents that the model can cite to generate a more accurate reply. Each document is a string-string dictionary.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/// A relevant documents that the model can cite to generate a more accurate reply. Each document is a string-string dictionary. | |
/// </summary> | |
/// Relevant documents that the model can cite to generate a more accurate reply. Each document is a string-string dictionary. | |
/// </summary> |
|
||
/// <summary> | ||
/// Relevant information that could be used by the model to generate a more accurate reply.<br/> | ||
/// The content of each document are generally short (should be under 300 words). Metadata should be used to provide additional information, both the key name and the value will be<br/> |
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 grammatical error in the documentation comment
There is a grammatical error in the documentation. The sentence should use "is" instead of "are":
- Incorrect: "The content of each document are generally short..."
- Correct: "The content of each document is generally short..."
Apply this diff to correct the error:
- /// The content of each document are generally short (should be under 300 words). Metadata should be used to provide additional information, both the key name and the value will be<br/>
+ /// The content of each document is generally short (should be under 300 words). Metadata should be used to provide additional information; both the key name and the value will be<br/>
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/// The content of each document are generally short (should be under 300 words). Metadata should be used to provide additional information, both the key name and the value will be<br/> | |
/// The content of each document is generally short (should be under 300 words). Metadata should be used to provide additional information; both the key name and the value will be<br/> |
return (global::Cohere.DocumentContentType)numValue; | ||
} | ||
default: | ||
throw new global::System.ArgumentOutOfRangeException(nameof(reader)); |
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.
Use JsonException
Instead of ArgumentOutOfRangeException
In the Read
method, throwing ArgumentOutOfRangeException
when encountering an unexpected JsonTokenType
is not appropriate, as the issue is due to invalid JSON content rather than an invalid argument. Consider throwing JsonException
to accurately represent parsing errors.
Suggested change:
- throw new global::System.ArgumentOutOfRangeException(nameof(reader));
+ throw new global::System.Text.Json.JsonException($"Unexpected token when parsing DocumentContentType: {reader.TokenType}");
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
throw new global::System.ArgumentOutOfRangeException(nameof(reader)); | |
throw new global::System.Text.Json.JsonException($"Unexpected token when parsing DocumentContentType: {reader.TokenType}"); |
global::Cohere.DocumentContentType value, | ||
global::System.Text.Json.JsonSerializerOptions options) | ||
{ | ||
writer = writer ?? throw new global::System.ArgumentNullException(nameof(writer)); |
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.
Simplify Null Check for writer
Parameter
Assigning back to the writer
parameter during the null check is unnecessary and unconventional. It is clearer to perform a standard null check without assignment.
Suggested change:
- writer = writer ?? throw new global::System.ArgumentNullException(nameof(writer));
+ if (writer == null)
+ {
+ throw new global::System.ArgumentNullException(nameof(writer));
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
writer = writer ?? throw new global::System.ArgumentNullException(nameof(writer)); | |
if (writer == null) | |
{ | |
throw new global::System.ArgumentNullException(nameof(writer)); | |
} |
throw new global::System.ArgumentOutOfRangeException(nameof(reader)); | ||
} | ||
|
||
return default; |
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.
Throw JsonException
When Deserialization Fails
Returning default
at the end of the Read
method can obscure deserialization issues by silently failing. It would be better to throw a JsonException
to inform the caller that the JSON input could not be deserialized into a valid DocumentContentType
.
Suggested change:
- return default;
+ throw new global::System.Text.Json.JsonException("Failed to deserialize DocumentContentType from JSON.");
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return default; | |
throw new global::System.Text.Json.JsonException("Failed to deserialize DocumentContentType from JSON."); |
var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.TextContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.TextContent> ?? | ||
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.TextContent).Name}"); |
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.
Refactor to eliminate duplicated code when retrieving TypeInfo
.
The logic for retrieving TypeInfo
is repeated multiple times for different types. This duplication can be reduced by extracting the retrieval into a helper method to improve maintainability and reduce potential errors.
Consider creating a private method to handle TypeInfo
retrieval:
private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<T> GetTypeInfo<T>(global::System.Text.Json.JsonSerializerOptions options)
{
var typeInfoResolver = options.TypeInfoResolver ?? throw new global::System.InvalidOperationException("TypeInfoResolver is not set.");
return typeInfoResolver.GetTypeInfo(typeof(T), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<T> ??
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(T).Name}");
}
Then, replace the repeated code with calls to this method:
-var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.TextContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.TextContent> ??
- throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.TextContent).Name}");
+var typeInfo = GetTypeInfo<global::Cohere.TextContent>(options);
Also applies to: 35-36, 50-51, 56-57, 75-76, 81-82
catch (global::System.Text.Json.JsonException) | ||
{ | ||
} |
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.
Avoid empty catch blocks to improve error handling.
Catching exceptions without any handling or logging can make debugging difficult and may suppress important errors.
Consider logging the exception or providing additional handling:
}
-catch (global::System.Text.Json.JsonException)
+catch (global::System.Text.Json.JsonException ex)
{
+ // Handle or log the exception as appropriate
+ // For example: Console.Error.WriteLine($"Deserialization failed: {ex.Message}");
}
If suppressing the exception is intentional, consider adding a comment to explain why:
catch (global::System.Text.Json.JsonException)
{
// Intentionally suppressing the exception to try deserializing into another type
}
Also applies to: 39-41
if (value.IsText) | ||
{ | ||
var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.TextContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.TextContent?> ?? | ||
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.TextContent).Name}"); | ||
global::System.Text.Json.JsonSerializer.Serialize(writer, value.Text, typeInfo); | ||
} | ||
else if (value.IsDocument) | ||
{ | ||
var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.DocumentContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.DocumentContent?> ?? | ||
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.DocumentContent).Name}"); | ||
global::System.Text.Json.JsonSerializer.Serialize(writer, value.Document, typeInfo); | ||
} | ||
} |
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.
Consolidate serialization logic to reduce code duplication.
The Write
method contains duplicated code for serializing TextContent
and DocumentContent
. Refactoring can improve code maintainability.
Consider extracting the serialization logic into a helper method:
private void SerializeContent<T>(global::System.Text.Json.Utf8JsonWriter writer, T content, global::System.Text.Json.JsonSerializerOptions options)
{
var typeInfo = GetTypeInfo<T>(options);
global::System.Text.Json.JsonSerializer.Serialize(writer, content, typeInfo);
}
Then, update the Write
method:
if (value.IsText)
{
- var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.TextContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.TextContent?> ??
- throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.TextContent).Name}");
- global::System.Text.Json.JsonSerializer.Serialize(writer, value.Text, typeInfo);
+ SerializeContent(writer, value.Text, options);
}
else if (value.IsDocument)
{
- var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.DocumentContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.DocumentContent?> ??
- throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.DocumentContent).Name}");
- global::System.Text.Json.JsonSerializer.Serialize(writer, value.Document, typeInfo);
+ SerializeContent(writer, value.Document, options);
}
var | ||
readerCopy = reader; |
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 variable declaration formatting.
There's an unnecessary line break between the var
keyword and the variable name, which affects readability.
Apply this diff to correct the formatting:
-var
-readerCopy = reader;
+var readerCopy = reader;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
var | |
readerCopy = reader; | |
var readerCopy = reader; |
if (text != null) | ||
{ | ||
var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.TextContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.TextContent> ?? | ||
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.TextContent).Name}"); | ||
_ = global::System.Text.Json.JsonSerializer.Deserialize(ref reader, typeInfo); | ||
} | ||
else if (document != null) | ||
{ | ||
var typeInfo = typeInfoResolver.GetTypeInfo(typeof(global::Cohere.DocumentContent), options) as global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Cohere.DocumentContent> ?? | ||
throw new global::System.InvalidOperationException($"Cannot get type info for {typeof(global::Cohere.DocumentContent).Name}"); | ||
_ = global::System.Text.Json.JsonSerializer.Deserialize(ref reader, typeInfo); | ||
} |
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.
Ensure the JSON reader advances correctly to prevent unexpected behavior.
Deserializing the JSON into _
after partial reads may lead to incorrect reader positions or duplicate processing.
Since readerCopy
is used for deserialization without advancing the original reader
, the original reader
remains at the start. Before returning, the reader
should be advanced to the end of the JSON token to prevent issues in subsequent processing.
Consider advancing the reader
appropriately:
...
}
-return result;
+// Advance the reader to the end of the current JSON token
+reader.Skip();
+return result;
Alternatively, you can perform deserialization once and use the result without re-reading:
...
}
-if (text != null)
-{
- // Already deserialized, no need to deserialize again
-}
-else if (document != null)
-{
- // Already deserialized, no need to deserialize again
-}
Committable suggestion was skipped due to low confidence.
Summary by CodeRabbit
New Features
documents
parameter to enhance chat interactions by allowing the model to reference relevant documents for more accurate replies.Document
class to encapsulate document content and metadata.Document
andToolContent
.Bug Fixes
Documents
property from theUserMessage
class to streamline data handling.Documentation