Update static functions in `pregel/index.ts` to match corresponding Python implementation by andrewnguonly · Pull Request #125 · langchain-ai/langgraphjs

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:45:58Z

Summary

This PR updates static functions in pregel/index.ts to the match the corresponding Python implementation. These functions are used in the core stream() and invoke() methods of the Pregel class.

Implementation

Implement _shouldInterrupt() (currently unused).
Update _applyWrites() to match Python implementation.
Remove _applyWritesFromView(). Removing the function did not break any tests. Not sure if this needs to be migrated.
Update _prepareNextTasks() to match Python implementation. Note: The side effect from this function is removed and the calling code is updated to ensure that all existing tests pass.
Implement _localRead() (replaced old implementation of read).
Update ChannelRead and ChannelWrite to extend from RunnableCallable (this was a regression).
Update @langchain/core and update RunnableCallable.invoke() to call mergeConfigs() (this was a regression).
Implement streamChannelsList getter.

Next Steps

Update _default(), stream(), and invoke() in Pregel class.

…ion + 1.

…iption>.

…tation.

…unit test for _prepareNextTasks().

…aneously. These changes need to be made together.

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:48:58Z

langgraph/src/channels/base.ts

  return newChannels;
 }

-export async function createCheckpoint<ValueType>(


This function needs to be non-async in order for _localRead() to be patched in RunnableConfig and eventually called so it doesn't return a Promise.

Need to double check if this is ok.

yep I think this doesn't have to be async

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:49:57Z

langgraph/src/pregel/index.ts

+
+      // Reassign nextCheckpoint to checkpoint because the subsequent implementation
+      // relies on side effects applied to checkpoint. Example: _applyWrites().
+      checkpoint = nextCheckpoint as Checkpoint;


This is temporary. When stream() is reimplemented, I think this code will be removed.

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:50:48Z

langgraph/src/pregel/index.ts

+      // A copy of the checkpoint is created because `checkpoint` is defined with `let`.
+      // `checkpoint` can be mutated during loop execution and when used in a function,
+      // may cause unintended consequences.
+      const checkpointCopy = copyCheckpoint(checkpoint);


This might be temporary. When stream() is reimplemented, this code might be removed.

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:51:46Z

langgraph/src/pregel/index.ts

        yield stepOutput;
-
-        if (typeof outputKeys !== "string") {
-          _applyWritesFromView(checkpoint, channels, stepOutput);


Removing this had no effect (i.e. all tests passed). Need to double check if I need to migrate it.

ah this is an api that used to exist in py but we removed, so good to remove here too

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:53:06Z

langgraph/src/pregel/index.ts

+    const newCheckpoint = createCheckpoint(checkpoint, channels);
+
+    // create a new copy of channels
+    const newChannels = Object.entries(channels).reduce(


Double check: This implementation is ported from a Python implementation that uses a context manager. What's the equivalent/preferred implementation in JS/TS?

this is fine

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:54:44Z

langgraph/src/pregel/read.ts

        triggers: this.triggers,
        mapper: this.mapper,
        writers: [...this.writers, coerceable as ChannelWrite],
-        bound: this.bound.pipe(coerceable),


This was a regression from a previous PR. It was only possible to do this once _prepareNextTasks() was updated.

Andrew Nguonly (andrewnguonly) · 2024-05-01T23:55:42Z

langgraph/src/utils.ts

@@ -1,4 +1,8 @@
-import { Runnable, RunnableConfig } from "@langchain/core/runnables";
+import {
+  mergeConfigs,


mergeConfigs() was exported in @langchain/core 0.1.61.

Nuno Campos (nfcampos) · 2024-05-02T00:10:13Z

langgraph/src/utils.ts

@@ -48,9 +52,13 @@ export class RunnableCallable extends Runnable {

    // TODO: mergeConfigs() from @langchain/core is not exported


remove comment

Nuno Campos (nfcampos) · 2024-05-02T00:10:39Z

package.json

  "author": "LangChain",
  "license": "MIT",
  "resolutions": {
-    "@langchain/core": "0.1.51"


why do we need this in both package json files?

Not 100% sure, but my assumption is because the @langchain/core dependencies are also needed in the examples directory, which is at the same level as langgraph.

Andrew Nguonly (andrewnguonly) added 11 commits April 30, 2024 13:54

Implement _shouldInterrupt().

a4cc122

Update _applyWrites(). Update checkpoint channel versions to max vers…

3f8bced

…ion + 1.

Remove _applyWritesFromView().

aac79f2

Update _prepareNextTasks() to support returning Array<PregelTaskDescr…

7b12b48

…iption>.

Update implementation of _prepareNextTasks() to match Python implemen…

50f5bc9

…tation.

Remove side effect from _prepareNextTasks() and migrate clients. Add …

ef3f381

…unit test for _prepareNextTasks().

Update implementation of _prepareTasks() and PregelNode.pipe() simult…

277117a

…aneously. These changes need to be made together.

Implement _localRead().

18a288e

Migrate ChannelRead and ChannelWrite to extend from RunnableCallable.

01d73ee

Update @langchain/core in order to use mergeConfigs().

0452396

Implement streamChannelsList getter.

bd81776

Andrew Nguonly (andrewnguonly) commented May 1, 2024

View reviewed changes

Andrew Nguonly (andrewnguonly) requested a review from Nuno Campos (nfcampos) May 1, 2024 23:56

Nuno Campos (nfcampos) reviewed May 2, 2024

View reviewed changes

Remove TODO comment.

bb6520e

Andrew Nguonly (andrewnguonly) requested a review from Nuno Campos (nfcampos) May 2, 2024 04:13

Nuno Campos (nfcampos) approved these changes May 2, 2024

View reviewed changes

Andrew Nguonly (andrewnguonly) merged commit a37e19d into langchain-ai:main May 2, 2024

Andrew Nguonly (andrewnguonly) deleted the migrate-pregel-3 branch May 2, 2024 15:14

		@@ -48,9 +52,13 @@ export class RunnableCallable extends Runnable {

		// TODO: mergeConfigs() from @langchain/core is not exported

Conversation

Andrew Nguonly (andrewnguonly) commented May 1, 2024

Summary

Implementation

Next Steps

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants