Skip to content
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

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

Closed
futugyou opened this issue May 20, 2023 · 11 comments
Assignees

Comments

@futugyou
Copy link

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

@craigomatic
Copy link
Contributor

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!

@tawalke
Copy link
Contributor

tawalke commented May 20, 2023

@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

@riccardodangelo
Copy link

@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");

@craigomatic
Copy link
Contributor

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

@SOE-YoungS
Copy link
Contributor

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.

@craigomatic
Copy link
Contributor

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

@aaron-puhl
Copy link

aaron-puhl commented Jun 5, 2023

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

@SOE-YoungS
Copy link
Contributor

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}");

@smandava
Copy link

smandava commented Jun 9, 2023

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

@craigomatic
Copy link
Contributor

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

@smandava
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants