fix(pinecone): fix PineconeStore to support null or undefined pageContent #6920
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
dosubot
bot
added
size:M
sam-trost
changed the title
sam-trost
changed the title
| @@ -425,7 +425,7 @@ export class PineconeStore extends VectorStore { | |||
| documentsWithScores.push([ | |||
| new Document({ | |||
| id, | |||
| pageContent: pageContent.toString(), | |||
| pageContent: pageContent?.toString(), | |||
There was a problem hiding this comment.
Should this fallback to something like an empty string?
Having this unpopulated breaks the typing doesn't it? At the very least I think it'd cause problems downstream
There was a problem hiding this comment.
Yeah, safer to do as you mentioned.
FWIW, the Document constructor handles it currently. I'm happy to defer but would probably suggest the Document class should be responsible for determining what the default is.
langchainjs/langchain-core/src/documents/document.ts
Lines 59 to 60 in 26f0714
There was a problem hiding this comment.
Yeah you're right - but it's still required in the input so I'm surprised it passes typechecking
There was a problem hiding this comment.
Oh yeah, I see that now...I agree, I would have anticipated the typecheck to fail...
There was a problem hiding this comment.
I think it's subtly being typed as any in the client
jacoblee93
changed the title
|
Thank you! |
This change provides compatibility with vectorstores that may not have been populated by langchain and do not have the pageContent property defined or populated. Previous versions of this module supported this, so this is to provide backwards compatibility for those data pipelines & workloads.
Note that any new Documents added to the document store via langchain will ensure pageContent is set.
Fixes #6919