Skip to content

Commit

Permalink
fix(langgraph): Fix bug in getting state with managed values (#493)
Browse files Browse the repository at this point in the history
  • Loading branch information
bracesproul authored Sep 18, 2024
1 parent dc8b527 commit cc0099a
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 3 deletions.
16 changes: 16 additions & 0 deletions libs/langgraph/src/managed/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,19 @@ export function isConfiguredManagedValue(
}
return false;
}

/**
* No-op class used when getting state values, as managed values should never be returned
* in get state calls.
*/
export class NoopManagedValue extends ManagedValue {
call() {}

static async initialize(
config: RunnableConfig,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
_args?: any
): Promise<ManagedValue> {
return Promise.resolve(new NoopManagedValue(config));
}
}
21 changes: 18 additions & 3 deletions libs/langgraph/src/pregel/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
isConfiguredManagedValue,
ManagedValue,
ManagedValueMapping,
NoopManagedValue,
type ManagedValueSpec,
} from "../managed/base.js";
import { patchConfigurable } from "../utils.js";
Expand Down Expand Up @@ -316,7 +317,8 @@ export class Pregel<
this.channels as Record<string, BaseChannel>,
checkpoint
);
const { managed } = await this.prepareSpecs(config);
// Pass `skipManaged: true` as managed values should not be returned in get state calls.
const { managed } = await this.prepareSpecs(config, { skipManaged: true });

const nextTasks = _prepareNextTasks(
checkpoint,
Expand Down Expand Up @@ -351,7 +353,8 @@ export class Pregel<
if (!this.checkpointer) {
throw new GraphValueError("No checkpointer set");
}
const { managed } = await this.prepareSpecs(config);
// Pass `skipManaged: true` as managed values should not be returned in get state calls.
const { managed } = await this.prepareSpecs(config, { skipManaged: true });

for await (const saved of this.checkpointer.list(config, options)) {
const channels = emptyChannels(
Expand Down Expand Up @@ -668,7 +671,14 @@ export class Pregel<
return super.stream(input, options);
}

protected async prepareSpecs(config: RunnableConfig) {
protected async prepareSpecs(
config: RunnableConfig,
options?: {
// Equivalent to the `skip_context` option in Python, but renamed
// to `managed` since JS does not implement the `Context` class.
skipManaged?: boolean;
}
) {
const configForManaged = patchConfigurable(config, {
[CONFIG_KEY_STORE]: this.store,
});
Expand All @@ -678,6 +688,11 @@ export class Pregel<
for (const [name, spec] of Object.entries(this.channels)) {
if (isBaseChannel(spec)) {
channelSpecs[name] = spec;
} else if (options?.skipManaged) {
managedSpecs[name] = {
cls: NoopManagedValue,
params: { config: {} },
};
} else {
managedSpecs[name] = spec;
}
Expand Down
71 changes: 71 additions & 0 deletions libs/langgraph/src/tests/pregel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5299,4 +5299,75 @@ describe("Managed Values (context) can be passed through state", () => {
};
await app.invoke(null, config4);
});

it("can get state when state has shared values", async () => {
const nodeOne = (_: typeof AgentAnnotation.State) => {
return {
messages: [
{
role: "assistant",
content: "no-op",
},
],
sharedStateKey: {
data: {
value: "shared",
},
},
};
};

const nodeTwo = (_: typeof AgentAnnotation.State) => {
// no-op
return {};
};

const workflow = new StateGraph(AgentAnnotation)
.addNode("nodeOne", nodeOne)
.addNode("nodeTwo", nodeTwo)
.addEdge(START, "nodeOne")
.addEdge("nodeOne", "nodeTwo")
.addEdge("nodeTwo", END);

const app = workflow.compile({
store,
checkpointer,
interruptBefore: ["nodeTwo"],
});

const config: Record<string, Record<string, unknown>> = {
configurable: { thread_id: threadId, assistant_id: "a" },
};

// Execute the graph. This will run `nodeOne` which sets the shared value,
// then is interrupted before executing `nodeTwo`.
await app.invoke(
{
messages: [
{
role: "user",
content: "no-op",
},
],
},
config
);

// Remove the "assistant_id" from the config and attempt to fetch the state.
// Since a `noop` managed value class is used when getting state, it should work
// even though the shared value key is not present.
if (config.configurable.assistant_id) {
delete config.configurable.assistant_id;
}
// Expect it does not throw an error complaining that the `assistant_id` key
// is not found in the config.
expect(await app.getState(config)).toBeTruthy();

// Re-running without re-setting the `assistant_id` key in the config should throw an error.
await expect(app.invoke(null, config)).rejects.toThrow(/assistant_id/);

// Re-set the `assistant_id` key in the config and attempt to fetch the state.
config.configurable.assistant_id = "a";
await app.invoke(null, config);
});
});

0 comments on commit cc0099a

Please sign in to comment.