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
  • Loading branch information
benjamincburns committed Oct 25, 2024
1 parent 9068ddc commit d1bb0d1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 39 deletions.
20 changes: 11 additions & 9 deletions libs/checkpoint-sqlite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,9 @@ CREATE TABLE IF NOT EXISTS writes (

async getTuple(config: RunnableConfig): Promise<CheckpointTuple | undefined> {
this.setup();
const {
thread_id,
checkpoint_ns = "",
checkpoint_id,
} = config.configurable ?? {};
const thread_id = config.configurable?.thread_id;
const checkpoint_ns = config.configurable?.checkpoint_ns ?? "";
const checkpoint_id = config.configurable?.checkpoint_id;
const sql = checkpoint_id
? "SELECT\n" +
" thread_id,\n" +
Expand Down Expand Up @@ -434,6 +432,10 @@ 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.");
}
Expand All @@ -454,10 +456,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 @@ -472,7 +474,7 @@ CREATE TABLE IF NOT EXISTS writes (
return {
configurable: {
thread_id: config.configurable?.thread_id,
checkpoint_ns: config.configurable?.checkpoint_ns,
checkpoint_ns,
checkpoint_id: checkpoint.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 d1bb0d1

Please sign in to comment.