Skip to content

Commit

Permalink
fix(checkpoint-sqlite): don't return undefined namespace from put
Browse files Browse the repository at this point in the history
fixes #592
  • Loading branch information
benjamincburns committed Oct 25, 2024
1 parent cc6625c commit 193965c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 35 deletions.
22 changes: 17 additions & 5 deletions libs/checkpoint-sqlite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,18 @@ CREATE TABLE IF NOT EXISTS writes (
): Promise<RunnableConfig> {
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) {
Expand All @@ -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,
Expand All @@ -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,
},
};
Expand Down
1 change: 1 addition & 0 deletions libs/checkpoint-sqlite/src/tests/checkpoints.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ describe("SqliteSaver", () => {
expect(runnableConfig).toEqual({
configurable: {
thread_id: "1",
checkpoint_ns: "",
checkpoint_id: checkpoint1.id,
},
});
Expand Down
52 changes: 22 additions & 30 deletions libs/checkpoint-validation/src/spec/put.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,37 +177,29 @@ export function putTests<T extends BaseCheckpointSaver>(
});
});

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
Expand Down

0 comments on commit 193965c

Please sign in to comment.