semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

'JsonNumberHandlingAttribute' is only valid on a number or a collection of numbers when applied to a property or field.

Open futugyou opened this issue 2 years ago • 3 comments

Describe the bug 'JsonNumberHandlingAttribute' is only valid on a number or a collection of numbers when applied to a property or field.

To Reproduce Steps to reproduce the behavior: 1.QdrantVectorDbClient.cs Line-132

public async Task<QdrantVectorRecord?> GetVectorByPayloadIdAsync(...)
{
       ... 
       // Line-132
       var data = JsonSerializer.Deserialize<SearchVectorsResponse>(responseContent);
       ...
}

Expected behavior 'JsonNumberHandlingAttribute' is only valid on a number or a collection of numbers when applied to a property or field. See member 'Id' on type 'Microsoft.SemanticKernel.Connectors.Memory.Qdrant.Http.ApiSchema.SearchVectorsResponse+ScoredPoint'.

Screenshots If applicable, add screenshots to help explain your problem.

1

1

futugyou avatar May 20 '23 13:05 futugyou

Sorry about this, we brought in a PR last week that was supposed to allow both numbers and strings for that property (Qdrant allows this) and it appears to have caused a regression.

To unblock yourself, you can remove the JsonNumberHandling attribute from ScoredPoint:

image

We will take another pass at fixing this!

craigomatic avatar May 20 '23 14:05 craigomatic

@futugyou Are you writing a GUID for your Scored Point ID?

Qdrant only allows a valid GUID as string or an int. Would you mind sharing an example of your Vector record?

Cc. @craigomatic

tawalke avatar May 20 '23 16:05 tawalke

@tawalke putting a Guid as id, the error doesn't occurs:

var id1 = Guid.NewGuid().ToString();
var key1 = await kernel.Memory.SaveInformationAsync(MemoryCollectionName, id: id1, text: "british short hair");

riccardodangelo avatar May 22 '23 10:05 riccardodangelo

PR #1313 was merged and should have resolved this, if you have a moment to verify that would be appreciated

craigomatic avatar Jun 02 '23 23:06 craigomatic

This issue seems to still be occurring in "0.15.230531.5-preview". I've just updated to the latest release & with no other code changes, have just run into this issue.

Id was previously set using strings in our vector db.

IE: We're looping though a JSON list of QnA items and saving them into a collection. The filename in our case, is named to represent the team / process the QnA list targets.

await _memory.SaveInformationAsync("QnACollection", qnaItem, $"{file.Name}-q{i}");

Not sure if we need to change anything here, but my understanding of QDrant is that a string value or an int should be accepted.

SOE-YoungS avatar Jun 05 '23 12:06 SOE-YoungS

You may need to use a guid for the id see https://github.com/microsoft/semantic-kernel/issues/794#issuecomment-1533797386

The next step in improving qdrant support is to add guid validation when a string is used as the id

craigomatic avatar Jun 05 '23 13:06 craigomatic

The id in "SaveInformationAsync" is not used as the id in Qdrant. See in QdrantMemoryStore:

var existingRecord = await this._qdrantClient.GetVectorByPayloadIdAsync(
                    collectionName,
                    record.Metadata.Id,
                    cancellationToken: cancellationToken)
                .ConfigureAwait(false);

if (existingRecord != null)
{
    pointId = existingRecord.PointId;
}

where "record.Metadata.Id" is the id from SaveInformationAsync if i did not miss something. https://github.com/microsoft/semantic-kernel/blob/6a270273ee6a2bc27baaa9c8157416b7c10716a7/dotnet/src/Connectors/Connectors.Memory.Qdrant/QdrantMemoryStore.cs#LL383C3-L383C3

aaron-puhl avatar Jun 05 '23 13:06 aaron-puhl

You may need to use a guid for the id see #794 (comment)

The next step in improving qdrant support is to add guid validation when a string is used as the id

Ok, so fortunately I'm in a position I can clear the vector DB and rebuild the data stored.

However, the following code is still throwing this error in the latest nuget package.

var id = Guid.NewGuid().ToString();
await _memory.SaveInformationAsync("QnACollection", qnaItem, id, description: $"{file.Name}-q{i}");

SOE-YoungS avatar Jun 05 '23 15:06 SOE-YoungS

Looks like the

ed and should have resolved this, if you have a moment to verify that would be appre

@craigomatic, The copilot chat app sample is giving the same error. The project is using SK version 0.15.230531.5-preview. Please share if there is a work around for the copilot chat app sample to make with work with Qdrant.

Stack trace:

     at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
     at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
     at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
     at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo)
     at Microsoft.SemanticKernel.Connectors.Memory.Qdrant.QdrantVectorDbClient.GetVectorByPayloadIdAsync(String collectionName, String metadataId, Boolean withVector, CancellationToken cancellationToken)
     at Microsoft.SemanticKernel.Connectors.Memory.Qdrant.QdrantMemoryStore.ConvertFromMemoryRecordAsync(String collectionName, MemoryRecord record, CancellationToken cancellationToken)
     at Microsoft.SemanticKernel.Connectors.Memory.Qdrant.QdrantMemoryStore.UpsertAsync(String collectionName, MemoryRecord record, CancellationToken cancellationToken)
     at Microsoft.SemanticKernel.Memory.SemanticTextMemory.SaveInformationAsync(String collection, String text, String id, String description, String additionalMetadata, CancellationToken cancellationToken)
     at SemanticKernel.Service.CopilotChat.Controllers.DocumentImportController.ParseDocumentContentToMemoryAsync(IKernel kernel, String content, DocumentImportForm documentImportForm, String memorySourceId) in /workspaces/semantic-kernel/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs:line 213
     at SemanticKernel.Service.CopilotChat.Controllers.DocumentImportController.ImportDocumentAsync(IKernel kernel, DocumentImportForm documentImportForm) in /workspaces/semantic-kernel/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs:line 128
     at SemanticKernel.Service.CopilotChat.Controllers.DocumentImportController.ImportDocumentAsync(IKernel kernel, DocumentImportForm documentImportForm) in /workspaces/semantic-kernel/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs:line 133

smandava avatar Jun 09 '23 14:06 smandava

Try updating to the latest nuget that was just released: https://www.nuget.org/packages/Microsoft.SemanticKernel/0.15.230609.2-preview

craigomatic avatar Jun 09 '23 23:06 craigomatic

Try updating to the latest nuget that was just released: https://www.nuget.org/packages/Microsoft.SemanticKernel/0.15.230609.2-preview

Worked! Thank you.

smandava avatar Jun 10 '23 04:06 smandava