From 193965c924f920fb13a823da4ff67d7d1eaeb7a1 Mon Sep 17 00:00:00 2001 From: Ben Burns <803016+benjamincburns@users.noreply.github.com> Date: Sat, 26 Oct 2024 02:18:50 +1300 Subject: [PATCH 1/2] fix(checkpoint-sqlite): don't return undefined namespace from put fixes #592 --- libs/checkpoint-sqlite/src/index.ts | 22 ++++++-- .../src/tests/checkpoints.test.ts | 1 + libs/checkpoint-validation/src/spec/put.ts | 52 ++++++++----------- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/libs/checkpoint-sqlite/src/index.ts b/libs/checkpoint-sqlite/src/index.ts index 6740bb220..d069a181c 100644 --- a/libs/checkpoint-sqlite/src/index.ts +++ b/libs/checkpoint-sqlite/src/index.ts @@ -297,6 +297,18 @@ CREATE TABLE IF NOT EXISTS writes ( ): Promise { this.setup(); + const thread_id = config.configurable?.thread_id; + const checkpoint_ns = config.configurable?.checkpoint_ns ?? ""; + const parent_checkpoint_id = config.configurable?.checkpoint_id; + + if (!config.configurable) { + throw new Error("Empty configuration supplied."); + } + + if (!thread_id) { + throw new Error("Missing thread_id field in config.configurable."); + } + const [type1, serializedCheckpoint] = this.serde.dumpsTyped(checkpoint); const [type2, serializedMetadata] = this.serde.dumpsTyped(metadata); if (type1 !== type2) { @@ -305,10 +317,10 @@ CREATE TABLE IF NOT EXISTS writes ( ); } const row = [ - config.configurable?.thread_id?.toString(), - config.configurable?.checkpoint_ns, + thread_id, + checkpoint_ns, checkpoint.id, - config.configurable?.checkpoint_id, + parent_checkpoint_id, type1, serializedCheckpoint, serializedMetadata, @@ -322,8 +334,8 @@ CREATE TABLE IF NOT EXISTS writes ( return { configurable: { - thread_id: config.configurable?.thread_id, - checkpoint_ns: config.configurable?.checkpoint_ns, + thread_id, + checkpoint_ns, checkpoint_id: checkpoint.id, }, }; diff --git a/libs/checkpoint-sqlite/src/tests/checkpoints.test.ts b/libs/checkpoint-sqlite/src/tests/checkpoints.test.ts index 3cc088303..c5498b54d 100644 --- a/libs/checkpoint-sqlite/src/tests/checkpoints.test.ts +++ b/libs/checkpoint-sqlite/src/tests/checkpoints.test.ts @@ -60,6 +60,7 @@ describe("SqliteSaver", () => { expect(runnableConfig).toEqual({ configurable: { thread_id: "1", + checkpoint_ns: "", checkpoint_id: checkpoint1.id, }, }); diff --git a/libs/checkpoint-validation/src/spec/put.ts b/libs/checkpoint-validation/src/spec/put.ts index d4a731efa..71c7d2039 100644 --- a/libs/checkpoint-validation/src/spec/put.ts +++ b/libs/checkpoint-validation/src/spec/put.ts @@ -177,37 +177,29 @@ export function putTests( }); }); - it_skipForSomeModules(initializer.checkpointerName, { - // TODO: SqliteSaver stores with undefined namespace instead of empty namespace - // see: https://github.com/langchain-ai/langgraphjs/issues/592 - "@langchain/langgraph-checkpoint-sqlite": - "TODO: SqliteSaver stores config with no checkpoint_ns instead of default namespace", - })( - "should default to empty namespace if the checkpoint namespace is missing from config.configurable", - async () => { - const missingNamespaceConfig: RunnableConfig = { - configurable: { thread_id }, - }; - - const { checkpoint, metadata } = initialCheckpointTuple({ - thread_id, - checkpoint_id: checkpoint_id1, - checkpoint_ns: "", - }); + it("should default to empty namespace if the checkpoint namespace is missing from config.configurable", async () => { + const missingNamespaceConfig: RunnableConfig = { + configurable: { thread_id }, + }; + + const { checkpoint, metadata } = initialCheckpointTuple({ + thread_id, + checkpoint_id: checkpoint_id1, + checkpoint_ns: "", + }); - const returnedConfig = await checkpointer.put( - missingNamespaceConfig, - checkpoint, - metadata!, - {} - ); - - expect(returnedConfig).not.toBeUndefined(); - expect(returnedConfig?.configurable).not.toBeUndefined(); - expect(returnedConfig?.configurable?.checkpoint_ns).not.toBeUndefined(); - expect(returnedConfig?.configurable?.checkpoint_ns).toEqual(""); - } - ); + const returnedConfig = await checkpointer.put( + missingNamespaceConfig, + checkpoint, + metadata!, + {} + ); + + expect(returnedConfig).not.toBeUndefined(); + expect(returnedConfig?.configurable).not.toBeUndefined(); + expect(returnedConfig?.configurable?.checkpoint_ns).not.toBeUndefined(); + expect(returnedConfig?.configurable?.checkpoint_ns).toEqual(""); + }); it_skipForSomeModules(initializer.checkpointerName, { // TODO: all of the checkpointers below store full channel_values on every put, rather than storing deltas From 81b2972d8f6d622e2890ebd38b2560882fefc61c Mon Sep 17 00:00:00 2001 From: jacoblee93 Date: Tue, 29 Oct 2024 14:00:15 -0700 Subject: [PATCH 2/2] Style nit --- libs/checkpoint-sqlite/src/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/checkpoint-sqlite/src/index.ts b/libs/checkpoint-sqlite/src/index.ts index 447754136..b5f521126 100644 --- a/libs/checkpoint-sqlite/src/index.ts +++ b/libs/checkpoint-sqlite/src/index.ts @@ -405,8 +405,10 @@ CREATE TABLE IF NOT EXISTS writes ( const checkpoint_ns = config.configurable?.checkpoint_ns ?? ""; const parent_checkpoint_id = config.configurable?.checkpoint_id; - if (!config.configurable?.thread_id) { - throw new Error("Missing thread_id field in config.configurable."); + if (!thread_id) { + throw new Error( + `Missing "thread_id" field in passed "config.configurable".` + ); } const preparedCheckpoint: Partial = copyCheckpoint(checkpoint);